Enable quidem tests to use different suppliers #16382

kgyrtkirk · 2024-05-03T10:33:21Z

enable quidem uri support for druidtest:///?ComponentSupplier=Nested and similar
changes the way SqlTestFrameworkConfig is being applied; all options will have their own annotation (its kinda impossible to detect that an annotation has a set value or its the default)
enables hierarchical processing of config annotation (was needed to enable class level supplier annotation)
moves uri processing related string2config stuff into SqlTestFrameworkConfig

* adds a test scoped jdbc driver for `druidtest:///` backed `DruidAvaticaTestDriver` * `DecoupledTestConfig` can switch test validation to be backed by a quidem execution * this enables plan/etc validation for those cases as well * `DruidQuidemTestBase` can be used to create module level set of quidem tests * added quidem commands: `!nativePlan`, `!logicalPlan`, `!druidPlan`, `!convertedPlan`

…m-runner-use-suppliers

This reverts commit 0908e7c.

…uppliers

rohangarg

Thank you so much for the changes @kgyrtkirk!
I have left a few comments, please let me know your thoughts on it.

sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfig.java

rohangarg · 2024-05-06T14:13:49Z

sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfig.java

+
+  private Object getQueryComponentSupplierForName(String name)
+  {
+    Set<Class<? extends QueryComponentSupplier>> subTypes = new Reflections("org.apache.druid")


this would not support a component supplier defined in an extension which has a proprietary package name.
also, we should cache this set probably

yes; only classes under the org.apache.druid will be considered - what do you suggest?

use ServiceLoader to register implementations - I would prefer this ; but seems unnecessarily complication

scan all packages?

something else?

I've replaced it with the following logic:

introduced a LoadingCache which is keyed by packageName

lookup first tries the org.apache.druid.sql.calcite package

falls back to asks for the root package - and if there are no matches even there -> it returns with an error

The current implementation of using a chained method with calcite and root package, along with the caching looks good to me 👍 One more thing to look at for performance changes could be to check the runtimes of SQL module tests in master vs this PR - hopefully they should be pretty close.

this newly added by-name lookup will not interfere with existing tests; as in those cases a ComponentSupplier class is specified as a java class explicitly in the annotation - however in case it comes from a jdbc-uri it will need the lookup.

run all Calcite*Test on master and on this branch - didn't seen any relevant difference in the runtimes

testrun on master

[INFO] ------------------------------------------------------- [INFO] T E S T S [INFO] ------------------------------------------------------- [INFO] Running org.apache.druid.sql.calcite.CalciteNestedDataQueryTest [INFO] Tests run: 145, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 9.255 s -- in org.apache.druid.sql.calcite.CalciteNestedDataQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteArraysQueryTest [INFO] Tests run: 156, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.42 s -- in org.apache.druid.sql.calcite.CalciteArraysQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteUnionQueryTest [INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.881 s -- in org.apache.druid.sql.calcite.CalciteUnionQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteSysQueryTest [WARN] Tests run: 4, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 2.646 s -- in org.apache.druid.sql.calcite.CalciteSysQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteLookupFunctionQueryTest [INFO] Tests run: 90, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.361 s -- in org.apache.druid.sql.calcite.CalciteLookupFunctionQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteStrictInsertTest [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.488 s -- in org.apache.druid.sql.calcite.CalciteStrictInsertTest [INFO] Running org.apache.druid.sql.calcite.CalciteSimpleQueryTest [INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.734 s -- in org.apache.druid.sql.calcite.CalciteSimpleQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteParameterQueryTest [INFO] Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.874 s -- in org.apache.druid.sql.calcite.CalciteParameterQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteJoinQueryTest [WARN] Tests run: 586, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 16.89 s -- in org.apache.druid.sql.calcite.CalciteJoinQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteCatalogInsertTest [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.637 s -- in org.apache.druid.sql.calcite.CalciteCatalogInsertTest [INFO] Running org.apache.druid.sql.calcite.CalciteExplainQueryTest [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.514 s -- in org.apache.druid.sql.calcite.CalciteExplainQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteExportTest [WARN] Tests run: 13, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 2.472 s -- in org.apache.druid.sql.calcite.CalciteExportTest [INFO] Running org.apache.druid.sql.calcite.CalciteWindowQueryTest [WARN] Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 4.641 s -- in org.apache.druid.sql.calcite.CalciteWindowQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteTableAppendTest [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.469 s -- in org.apache.druid.sql.calcite.CalciteTableAppendTest [INFO] Running org.apache.druid.sql.calcite.CalciteReplaceDmlTest [INFO] Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.769 s -- in org.apache.druid.sql.calcite.CalciteReplaceDmlTest [INFO] Running org.apache.druid.sql.calcite.planner.CalcitePlannerModuleTest [INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.435 s -- in org.apache.druid.sql.calcite.planner.CalcitePlannerModuleTest [INFO] Running org.apache.druid.sql.calcite.planner.CalcitesTest [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 s -- in org.apache.druid.sql.calcite.planner.CalcitesTest [INFO] Running org.apache.druid.sql.calcite.CalciteCorrelatedQueryTest [INFO] Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.919 s -- in org.apache.druid.sql.calcite.CalciteCorrelatedQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteQueryTest [WARN] Tests run: 389, Failures: 0, Errors: 0, Skipped: 4, Time elapsed: 13.97 s -- in org.apache.druid.sql.calcite.CalciteQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteSubqueryTest [INFO] Tests run: 44, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.449 s -- in org.apache.druid.sql.calcite.CalciteSubqueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteInsertDmlTest [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.959 s -- in org.apache.druid.sql.calcite.CalciteInsertDmlTest [INFO] Running org.apache.druid.sql.calcite.CalciteSelectQueryTest [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.949 s -- in org.apache.druid.sql.calcite.CalciteSelectQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteMultiValueStringQueryTest [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.020 s -- in org.apache.druid.sql.calcite.CalciteMultiValueStringQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteCatalogReplaceTest [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.566 s -- in org.apache.druid.sql.calcite.CalciteCatalogReplaceTest [INFO] Running org.apache.druid.sql.calcite.CalciteTimeBoundaryQueryTest [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.472 s -- in org.apache.druid.sql.calcite.CalciteTimeBoundaryQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteScanSignatureTest [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.472 s -- in org.apache.druid.sql.calcite.CalciteScanSignatureTest [INFO] [INFO] Results: [INFO] [WARN] Tests run: 1845, Failures: 0, Errors: 0, Skipped: 11 [INFO] [INFO] [INFO] --- animal-sniffer-maven-plugin:1.23:check (check-java-api) @ druid-sql --- [INFO] Checking unresolved references to org.codehaus.mojo.signature:java18:1.0 [INFO] Segment walltime 124 s, segment projects service time 124 s, effective/maximum degree of concurrency 1.00/11 [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 02:04 min (Wall Clock) [INFO] Finished at: 2024-05-08T10:54:19Z [INFO] ------------------------------------------------------------------------

testrun on branch

[INFO] ------------------------------------------------------- [INFO] T E S T S [INFO] ------------------------------------------------------- [INFO] Running org.apache.druid.sql.calcite.CalciteNestedDataQueryTest [INFO] Tests run: 145, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 9.195 s -- in org.apache.druid.sql.calcite.CalciteNestedDataQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteArraysQueryTest [INFO] Tests run: 156, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.17 s -- in org.apache.druid.sql.calcite.CalciteArraysQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteUnionQueryTest [INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.815 s -- in org.apache.druid.sql.calcite.CalciteUnionQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteSysQueryTest [WARN] Tests run: 4, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 2.628 s -- in org.apache.druid.sql.calcite.CalciteSysQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteLookupFunctionQueryTest [INFO] Tests run: 90, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.194 s -- in org.apache.druid.sql.calcite.CalciteLookupFunctionQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteStrictInsertTest [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.519 s -- in org.apache.druid.sql.calcite.CalciteStrictInsertTest [INFO] Running org.apache.druid.sql.calcite.CalciteSimpleQueryTest [INFO] Tests run: 12, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.774 s -- in org.apache.druid.sql.calcite.CalciteSimpleQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteParameterQueryTest [INFO] Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.923 s -- in org.apache.druid.sql.calcite.CalciteParameterQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteJoinQueryTest [WARN] Tests run: 586, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 16.70 s -- in org.apache.druid.sql.calcite.CalciteJoinQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteCatalogInsertTest [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.681 s -- in org.apache.druid.sql.calcite.CalciteCatalogInsertTest [INFO] Running org.apache.druid.sql.calcite.CalciteExplainQueryTest [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.536 s -- in org.apache.druid.sql.calcite.CalciteExplainQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteExportTest [WARN] Tests run: 13, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 2.483 s -- in org.apache.druid.sql.calcite.CalciteExportTest [INFO] Running org.apache.druid.sql.calcite.CalciteWindowQueryTest [WARN] Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 4.509 s -- in org.apache.druid.sql.calcite.CalciteWindowQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteTableAppendTest [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.490 s -- in org.apache.druid.sql.calcite.CalciteTableAppendTest [INFO] Running org.apache.druid.sql.calcite.CalciteReplaceDmlTest [INFO] Tests run: 46, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.813 s -- in org.apache.druid.sql.calcite.CalciteReplaceDmlTest [INFO] Running org.apache.druid.sql.calcite.planner.CalcitePlannerModuleTest [INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.428 s -- in org.apache.druid.sql.calcite.planner.CalcitePlannerModuleTest [INFO] Running org.apache.druid.sql.calcite.planner.CalcitesTest [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.001 s -- in org.apache.druid.sql.calcite.planner.CalcitesTest [INFO] Running org.apache.druid.sql.calcite.CalciteCorrelatedQueryTest [INFO] Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 4.061 s -- in org.apache.druid.sql.calcite.CalciteCorrelatedQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteQueryTest [WARN] Tests run: 389, Failures: 0, Errors: 0, Skipped: 4, Time elapsed: 14.12 s -- in org.apache.druid.sql.calcite.CalciteQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteSubqueryTest [INFO] Tests run: 44, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.475 s -- in org.apache.druid.sql.calcite.CalciteSubqueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteInsertDmlTest [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.018 s -- in org.apache.druid.sql.calcite.CalciteInsertDmlTest [INFO] Running org.apache.druid.sql.calcite.CalciteSelectQueryTest [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.973 s -- in org.apache.druid.sql.calcite.CalciteSelectQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteMultiValueStringQueryTest [INFO] Tests run: 59, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.062 s -- in org.apache.druid.sql.calcite.CalciteMultiValueStringQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteCatalogReplaceTest [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.681 s -- in org.apache.druid.sql.calcite.CalciteCatalogReplaceTest [INFO] Running org.apache.druid.sql.calcite.CalciteTimeBoundaryQueryTest [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.577 s -- in org.apache.druid.sql.calcite.CalciteTimeBoundaryQueryTest [INFO] Running org.apache.druid.sql.calcite.CalciteScanSignatureTest [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.448 s -- in org.apache.druid.sql.calcite.CalciteScanSignatureTest [INFO] [INFO] Results: [INFO] [WARN] Tests run: 1845, Failures: 0, Errors: 0, Skipped: 11 [INFO] [INFO] [INFO] --- animal-sniffer-maven-plugin:1.23:check (check-java-api) @ druid-sql --- [INFO] Checking unresolved references to org.codehaus.mojo.signature:java18:1.0 [INFO] Segment walltime 126 s, segment projects service time 126 s, effective/maximum degree of concurrency 1.00/11 [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS [INFO] ------------------------------------------------------------------------ [INFO] Total time: 02:06 min (Wall Clock) [INFO] Finished at: 2024-05-08T10:52:09Z [INFO] ------------------------------------------------------------------------

ah, I see. Sorry I missed that! Although, the few quidem tests we have right now would invoke the lookup right?
Also, thanks for checking the time difference between the runtimes

yes - they use this method; and they will benefit from these caches more-and-more as we will get more cases!

sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfig.java

sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfigTest.java

This reverts commit a52fba5.

This reverts commit 2051dbc.

sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfig.java

rohangarg

Thanks for the updates @kgyrtkirk
LGTM 👍

Approving the changes, but waiting for your response on a couple of comments before merging

* enable quidem uri support for `druidtest:///?ComponentSupplier=Nested` and similar * changes the way `SqlTestFrameworkConfig` is being applied; all options will have their own annotation (its kinda impossible to detect that an annotation has a set value or its the default) * enables hierarchical processing of config annotation (was needed to enable class level supplier annotation) * moves uri processing related string2config stuff into `SqlTestFrameworkConfig`

kgyrtkirk added 30 commits April 9, 2024 11:45

add

8cf12ce

aa

9040d94

Make QueryComponentSupliers independent from test classes

3b7d7a8

more wood

d93c499

renames

05439eb

remove

038bbfd

save logical plan

bfefc06

updates

02e7e96

remove

d92fec3

updates

6f547e1

Merge branch 'quidem-runner' into quidem-runner-tx

18ba40c

Merge remote-tracking branch 'apache/master' into extract-suppliers

3d899aa

Merge remote-tracking branch 'kgyrtkirk/extract-suppliers' into quide…

2aea364

…m-runner-use-suppliers

fix style

6f59c13

add supplier

0d518c2

make supplier test work

d7ea8eb

better error

6514c30

update

b06b97e

some tries to fix msq

1ed91d4

update quidem cmdline

d4488e1

update

9db1aee

add w

4b26f4e

missed update

3bc928e

disable quidem tests in sqlcompatible=false

b637d53

make join opts named

f840e9f

some stuff

b0a9991

add iq for join stuff

8b46032

undo not needed change

38471a6

remove&undo quidem run in decoupledextension for now

d5bf196

kgyrtkirk added 9 commits May 3, 2024 11:36

intellij warn

1e7ee03

unused

049a0ba

use correct dep

04f7b06

one-less

023715f

one-less

0908e7c

Revert "one-less"

7585773

This reverts commit 0908e7c.

one-less

4b3a02d

Merge remote-tracking branch 'apache/master' into quidem-runner-use-s…

2adddb5

…uppliers

undo integ test pom.xml

5670202

rohangarg reviewed May 6, 2024

View reviewed changes

kgyrtkirk added 15 commits May 7, 2024 06:55

fix eclipse compile

9ae6a3d

rename annotation supplier => componentSupplier

244d599

add testcase

829fdcb

trial

a52fba5

Revert "trial"

11b5489

This reverts commit a52fba5.

remove check; add @nonnull

29d4ba9

add cached load

f3d0980

cleanup

f55dc29

cleanup

c46a65c

update exceptiuon

2051dbc

Revert "update exceptiuon"

58ef686

This reverts commit 2051dbc.

rx

1d7e406

use identity

e4d687c

update to use processor sublclass

5e079df

clenaup/etc

d8baa84

github-advanced-security bot found potential problems May 7, 2024

View reviewed changes

sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfig.java Fixed Show fixed Hide fixed

sql/src/test/java/org/apache/druid/sql/calcite/SqlTestFrameworkConfig.java Fixed Show fixed Hide fixed

fix warnings

b8967e1

rohangarg approved these changes May 8, 2024

View reviewed changes

kgyrtkirk merged commit 1811674 into apache:master May 9, 2024
88 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable quidem tests to use different suppliers #16382

Enable quidem tests to use different suppliers #16382

kgyrtkirk commented May 3, 2024 •

edited

Loading

rohangarg left a comment

rohangarg May 6, 2024

kgyrtkirk May 7, 2024

rohangarg May 8, 2024

kgyrtkirk May 8, 2024

rohangarg May 9, 2024

kgyrtkirk May 9, 2024

rohangarg left a comment

Enable quidem tests to use different suppliers #16382

Enable quidem tests to use different suppliers #16382

Conversation

kgyrtkirk commented May 3, 2024 • edited Loading

rohangarg left a comment

Choose a reason for hiding this comment

rohangarg May 6, 2024

Choose a reason for hiding this comment

kgyrtkirk May 7, 2024

Choose a reason for hiding this comment

rohangarg May 8, 2024

Choose a reason for hiding this comment

kgyrtkirk May 8, 2024

Choose a reason for hiding this comment

rohangarg May 9, 2024

Choose a reason for hiding this comment

kgyrtkirk May 9, 2024

Choose a reason for hiding this comment

rohangarg left a comment

Choose a reason for hiding this comment

kgyrtkirk commented May 3, 2024 •

edited

Loading