Skip to content

Add native analyze e2e tests#20028

Closed
karteekmurthys wants to merge 2 commits intoprestodb:masterfrom
karteekmurthys:analyze-e2e-native
Closed

Add native analyze e2e tests#20028
karteekmurthys wants to merge 2 commits intoprestodb:masterfrom
karteekmurthys:analyze-e2e-native

Conversation

@karteekmurthys
Copy link
Contributor

@karteekmurthys karteekmurthys commented Jun 29, 2023

Added tests to verify ANALYZE command and compare stats between Prestissimo and Presto for the test tables.
Resolves this #19922

@karteekmurthys karteekmurthys requested a review from a team as a code owner June 29, 2023 16:17
@karteekmurthys karteekmurthys self-assigned this Jun 29, 2023
@karteekmurthys karteekmurthys marked this pull request as draft June 29, 2023 16:17
@karteekmurthys
Copy link
Contributor Author

The createJavaQueryRunner() is failing with following exceptions:

[ERROR] Failures: 
[ERROR]   TestPrestoNativeAggregations>AbstractTestQueryFramework.init:84->createExpectedQueryRunner:31 ? Creation Unable to create injector, see the following errors:

1) Configuration property 'hive.allow-drop-table' was not used
  at com.facebook.airlift.bootstrap.Bootstrap.lambda$initialize$2(Bootstrap.java:244)

1 error
[ERROR]   TestPrestoNativeJoinQueries>AbstractTestQueryFramework.init:84->createExpectedQueryRunner:31 ? Creation Unable to create injector, see the following errors:

1) Configuration property 'hive.allow-drop-table' was not used
  at com.facebook.airlift.bootstrap.Bootstrap.lambda$initialize$2(Bootstrap.java:244)

1 error
[ERROR]   TestPrestoNativeWindowQueries>AbstractTestQueryFramework.init:84->createExpectedQueryRunner:31 ? Creation Unable to create injector, see the following errors:

1) Configuration property 'hive.allow-drop-table' was not used
  at com.facebook.airlift.bootstrap.Bootstrap.lambda$initialize$2(Bootstrap.java:244)

Will try to add hive.allow-drop-table property to the config and test it out.

@karteekmurthys karteekmurthys marked this pull request as ready for review June 29, 2023 19:58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"nation" is a very tiny table for this kind of test. But its probably sufficient to test the correctness of this work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth adding a comment about the results returned by ANALYZE for this query. Is one of the arrays an empty result ?

Copy link
Contributor Author

@karteekmurthys karteekmurthys Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont' think so. Here are the partitions would look like:

presto:tpch> select * from "tmpregion$partitions";
 regionkey | nationkey 
-----------+-----------
         2 |        12 
         2 |         8 
         2 |         9 
         2 |        18 
         2 |        21 
         4 |        13 
         4 |        11 
         4 |        20 
         4 |        10 
         4 |         4 
         3 |        23 
         3 |        22 
         3 |         6 
         3 |         7 
         3 |        19 
         1 |        24 
         1 |         1 
         1 |        17 
         1 |         2 
         1 |         3 
         0 |        15 
         0 |        14 
         0 |         0 
         0 |        16 
         0 |         5 
(25 rows)

Added the output in the comments.

@aditi-pandit
Copy link
Contributor

Please rebase with #20025 and check.

@karteekmurthys karteekmurthys requested review from a team and aditi-pandit June 29, 2023 21:04
@karteekmurthys
Copy link
Contributor Author

Please rebase with #20025 and check.

Yes, I rebased on it. The tests passed after that. This time hit new failure: #20029

@karteekmurthys karteekmurthys marked this pull request as draft June 30, 2023 04:25
@karteekmurthys
Copy link
Contributor Author

There seems to be an aggregate function missing required by analyze command:

Caused by: java.lang.RuntimeException:  Aggregate function not registered: presto.default.$internal$max_data_size_for_stats
	at com.facebook.presto.tests.AbstractTestingPrestoClient.execute(AbstractTestingPrestoClient.java:124)
	at com.facebook.presto.tests.DistributedQueryRunner.execute(DistributedQueryRunner.java:733)
	at com.facebook.presto.tests.DistributedQueryRunner.execute(DistributedQueryRunner.java:701)
	at com.facebook.presto.tests.QueryAssertions.assertQuery(QueryAssertions.java:175)
	... 19 more

When I test basic table with a single integer column, analyze works fine on native cluster but if the table has a varchar column, analyze command fails. Looks like I need to create a table with multiple column types like Dates, Varchars, Ints, floats and decimals.

presto:kart> insert into t1 values(1),(2),(3),(4);
INSERT: 4 rows
presto:kart> analyze t1;
ANALYZE: 4 rows



presto:kart> insert into t2 values('a'),('b');
INSERT: 2 rows

Query 20230630_042753_00018_2h47r, FINISHED, 2 nodes
Splits: 2 total, 2 done (100.00%)
[Latency: client-side: 0:01, server-side: 0:01] [0 rows, 0B] [0 rows/s, 0B/s]

presto:kart> analyze t2;
Query 20230630_042758_00019_2h47r failed:  Aggregate function not registered: presto.default.$internal$max_data_size_for_stats

@karteekmurthys
Copy link
Contributor Author

Closing this PR. Adding tests as part of this #20055

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants