SQL: [Tests] Add integ tests for selecting a literal and an aggregate #42121

emasab · 2019-05-13T16:58:52Z

No description provided.

emasab · 2019-05-13T17:03:35Z

Closes #41411

elasticmachine · 2019-05-15T01:01:03Z

Pinging @elastic/es-search

tvernum · 2019-05-15T01:01:18Z

@elasticmachine test this please

emasab · 2019-05-18T18:31:23Z

... otherwise this query was failing because the literal with this commit is supportedByAggsOnlyQuery

SELECT DAY_NAME(birth_date + INTERVAL 1 DAY) FROM test_emp

emasab · 2019-09-15T16:31:54Z

the latest commit fixes some integration tests that were failing. Pending to add more unit/integration tests, at the moment there is only one unit test for the issue that is addressing

In a previous version that I've not sent I had left both supportedByAggsQuery and supportedByNoAggsQuery but given that the not aggregated fields inside an aggregation query are detected in the verification phase, another check should not be necessary. But if you want to maintain it, I could restore. The check was like:

    public boolean isAggsOnly() {
        if (aggsOnly == null) {
            aggsOnly = Boolean.valueOf(this.fields.stream().allMatch(t -> t.v1().supportedByAggsQuery())) &&
!Boolean.valueOf(this.fields.stream().allMatch(t -> t.v1().supportedByNoAggsQuery()));
        }

        return aggsOnly.booleanValue();
    }

in that case the ConstantInput was the only that was supportedByAggsQuery and supportedByNoAggsQuery. The integration tests were successful too with that approach.

tvernum · 2019-09-18T08:33:39Z

@elastic/es-sql Can someone look at this community PR?

emasab · 2019-09-20T18:26:43Z

rebased on current master

emasab · 2019-11-01T20:30:26Z

rebased

emasab · 2019-11-10T10:35:36Z

rebased, some formatting and other integration tests

emasab · 2019-11-19T18:57:16Z

rebased

matriv · 2020-02-04T16:10:26Z

Hi @emasab

Apologies for the delay here. Since the merging of #49570 seems that the issue is solved.
It would be great though to add the tests that you implemented to have this behaviour with
literals and aggregations tested thoroughly. Would you mind adjusting your PR to only add the
new tests?

Thank you!

emasab · 2020-02-05T17:52:49Z

Hi @matriv, happy to hear from you! I'm going to do that asap.

Best

matriv · 2020-02-05T23:42:49Z

@emasab Thank you!

emasab · 2020-02-06T23:09:51Z

That's it! Only the tests are left and they're passing.

matriv

LGTM, left a comment regarding the unit test.

matriv · 2020-02-06T23:33:47Z

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java

@@ -430,4 +430,17 @@ public void testFoldingOfPivot() {
        assertThat(a, containsString("\"terms\":{\"field\":\"keyword\""));
        assertThat(a, containsString("{\"avg\":{\"field\":\"int\"}"));
    }
+
+    public void testFoldingOfLiteralWithGroupBy() {


You can remove this, as it's already added here:

elasticsearch/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java

Line 383 in 26283b6

p = plan("SELECT 1, count(*) FROM test GROUP BY int");

matriv · 2020-02-06T23:34:44Z

@astefan Could you please also take a look?

emasab · 2020-02-07T15:33:00Z

@matriv I've removed the duplicated unit test

astefan

LGTM

matriv · 2020-02-07T16:30:35Z

@elasticmachine ok to test

matriv · 2020-02-07T17:26:33Z

@emasab Thanks a lot for your patience and your contribution!

…#42121) The related issue regarding aggregation queries where some literals are also selected together with aggregate function has been fixed with #49570. Add integration tests to verify the behavior. Relates to: #41411 (cherry picked from commit 9f414a8)

Add some more tests where more than one literal is selected, unaliased and aliased. Follows: elastic#42121

Add some more tests where more than one literal is selected, unaliased and aliased. Follows: #42121

Add some more tests where more than one literal is selected, unaliased and aliased. Follows: #42121 (cherry picked from commit 405271d)

tvernum added the :Analytics/SQL SQL querying label May 15, 2019

emasab changed the title ~~bugfix for https://github.com/elastic/elasticsearch/issues/41411~~ SQL: selecting a literal and an aggregate generates an error Jun 10, 2019

emasab force-pushed the bugfix/41411-selecting-a-literal branch from 996302d to a8986fb Compare June 12, 2019 16:19

astefan requested review from costin, matriv and astefan September 18, 2019 08:41

emasab force-pushed the bugfix/41411-selecting-a-literal branch from 0bd8e98 to 4b2e482 Compare September 20, 2019 18:26

emasab force-pushed the bugfix/41411-selecting-a-literal branch from 4b2e482 to 17721c7 Compare September 25, 2019 21:04

emasab force-pushed the bugfix/41411-selecting-a-literal branch from 17721c7 to 311ff60 Compare November 1, 2019 20:29

emasab force-pushed the bugfix/41411-selecting-a-literal branch from 311ff60 to f5a07cc Compare November 10, 2019 10:34

emasab force-pushed the bugfix/41411-selecting-a-literal branch from 788eefa to e683dc4 Compare November 19, 2019 18:56

matriv mentioned this pull request Feb 4, 2020

SQL: [Tests] add tests for literals and GROUP BY #51878

Merged

emasab force-pushed the bugfix/41411-selecting-a-literal branch 2 times, most recently from 57e3436 to 26283b6 Compare February 6, 2020 23:02

matriv approved these changes Feb 6, 2020

View reviewed changes

literals and aggregates unit and integration tests

61a4700

emasab force-pushed the bugfix/41411-selecting-a-literal branch from 26283b6 to 61a4700 Compare February 7, 2020 15:30

matriv added >test-failure Triaged test failures from CI v7.6.1 v7.7.0 >test Issues or PRs that are addressing/adding tests v8.0.0 and removed >test-failure Triaged test failures from CI labels Feb 7, 2020

astefan approved these changes Feb 7, 2020

View reviewed changes

matriv changed the title ~~SQL: selecting a literal and an aggregate generates an error~~ SQL: [Tests] Add integ tests for selecting a literal and an aggregate Feb 7, 2020

matriv merged commit 9f414a8 into elastic:master Feb 7, 2020

$@polyfractal$ polyfractal added v7.6.0 v7.6.1 and removed v7.6.1 v7.6.0 labels Feb 7, 2020

matriv added a commit to matriv/elasticsearch that referenced this pull request Feb 7, 2020

SQL: [Tests] Add more tests for aggs and literals

953c741

Add some more tests where more than one literal is selected, unaliased and aliased. Follows: elastic#42121

matriv mentioned this pull request Feb 7, 2020

SQL: [Tests] Add more tests for aggs and literals #52086

Merged

matriv added a commit that referenced this pull request Feb 9, 2020

SQL: [Tests] Add more tests for aggs and literals (#52086)

405271d

Add some more tests where more than one literal is selected, unaliased and aliased. Follows: #42121

matriv added a commit that referenced this pull request Feb 9, 2020

SQL: [Tests] Add more tests for aggs and literals (#52086)

3e7f939

Add some more tests where more than one literal is selected, unaliased and aliased. Follows: #42121 (cherry picked from commit 405271d)

matriv added a commit that referenced this pull request Feb 9, 2020

SQL: [Tests] Add more tests for aggs and literals (#52086)

7326708

Add some more tests where more than one literal is selected, unaliased and aliased. Follows: #42121 (cherry picked from commit 405271d)

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SQL: [Tests] Add integ tests for selecting a literal and an aggregate #42121

SQL: [Tests] Add integ tests for selecting a literal and an aggregate #42121

emasab commented May 13, 2019

emasab commented May 13, 2019

elasticmachine commented May 15, 2019

tvernum commented May 15, 2019

emasab commented May 18, 2019 •

edited

Loading

emasab commented Sep 15, 2019 •

edited

Loading

tvernum commented Sep 18, 2019

emasab commented Sep 20, 2019

emasab commented Nov 1, 2019

emasab commented Nov 10, 2019 •

edited

Loading

emasab commented Nov 19, 2019

matriv commented Feb 4, 2020

emasab commented Feb 5, 2020

matriv commented Feb 5, 2020

emasab commented Feb 6, 2020

matriv left a comment

matriv Feb 6, 2020

matriv commented Feb 6, 2020

emasab commented Feb 7, 2020

astefan left a comment

matriv commented Feb 7, 2020

matriv commented Feb 7, 2020

SQL: [Tests] Add integ tests for selecting a literal and an aggregate #42121

SQL: [Tests] Add integ tests for selecting a literal and an aggregate #42121

Conversation

emasab commented May 13, 2019

emasab commented May 13, 2019

elasticmachine commented May 15, 2019

tvernum commented May 15, 2019

emasab commented May 18, 2019 • edited Loading

emasab commented Sep 15, 2019 • edited Loading

tvernum commented Sep 18, 2019

emasab commented Sep 20, 2019

emasab commented Nov 1, 2019

emasab commented Nov 10, 2019 • edited Loading

emasab commented Nov 19, 2019

matriv commented Feb 4, 2020

emasab commented Feb 5, 2020

matriv commented Feb 5, 2020

emasab commented Feb 6, 2020

matriv left a comment

Choose a reason for hiding this comment

matriv Feb 6, 2020

Choose a reason for hiding this comment

matriv commented Feb 6, 2020

emasab commented Feb 7, 2020

astefan left a comment

Choose a reason for hiding this comment

matriv commented Feb 7, 2020

matriv commented Feb 7, 2020

emasab commented May 18, 2019 •

edited

Loading

emasab commented Sep 15, 2019 •

edited

Loading

emasab commented Nov 10, 2019 •

edited

Loading