Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add table statistics and automatic Join pushdown for SQL Server connector #11637

Merged
merged 3 commits into from Apr 8, 2022

Conversation

findepi
Copy link
Member

@findepi findepi commented Mar 23, 2022

No description provided.

@findepi findepi added enhancement New feature or request performance labels Mar 23, 2022
@findepi
Copy link
Member Author

findepi commented Mar 23, 2022

Currently based on #11635

@findepi findepi force-pushed the findepi/rdbms-stats-sqlserver branch from 02001be to f35810b Compare March 24, 2022 19:58
@cla-bot cla-bot bot added the cla-signed label Mar 24, 2022
@findepi
Copy link
Member Author

findepi commented Mar 24, 2022

rebased after #11635 merged

@Override
protected void checkEmptyTableStats(String tableName)
{
// TODO: Empty tables should have NDV as 0 and nulls fraction as 1 (https://starburstdata.atlassian.net/browse/SEP-5963)
Copy link
Member

Choose a reason for hiding this comment

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

link?

Copy link
Member Author

Choose a reason for hiding this comment

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

rmeoving link, leaving the comment

@findepi findepi force-pushed the findepi/rdbms-stats-sqlserver branch from f35810b to 4660968 Compare April 1, 2022 07:35
@findepi
Copy link
Member Author

findepi commented Apr 1, 2022

( just rebased )

@findepi findepi force-pushed the findepi/rdbms-stats-sqlserver branch from 4660968 to 0a2ae91 Compare April 1, 2022 07:41
@findepi findepi force-pushed the findepi/rdbms-stats-sqlserver branch from 2ca822a to 3045084 Compare April 6, 2022 12:09
@findepi
Copy link
Member Author

findepi commented Apr 6, 2022

In https://github.com/trinodb/trino/runs/5784397964?check_suite_focus=true the SQL Server tests crashed.

Project's default for air.test.parallel is 'methods'. By design, 'classes' makes TestNG run tests from one class in a single thread.
As a side effect, it prevents TestNG from initializing multiple test instances upfront, which happens with 'methods'.
A potential downside can be long tail single-threaded execution of a single long test class.
TODO (https://github.com/trinodb/trino/issues/11294) remove when we upgrade to surefire with https://issues.apache.org/jira/browse/SUREFIRE-1967
Copy link
Member Author

Choose a reason for hiding this comment

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

This is supposed to workaround OOM problems when multiple test instances
are initialized. This seems to be prerequisite if we want to add more
QueryRunner-based tests.
@findepi findepi force-pushed the findepi/rdbms-stats-sqlserver branch from 3045084 to 909b3ca Compare April 6, 2022 20:15
@findepi
Copy link
Member Author

findepi commented Apr 6, 2022

( just rebased, to resolve conflicts in ci.yml )

@findepi
Copy link
Member Author

findepi commented Apr 8, 2022

stress tests awesomely green; sqlserver x20

https://github.com/trinodb/trino/runs/5858695489?check_suite_focus=true

@findepi findepi force-pushed the findepi/rdbms-stats-sqlserver branch from c16a613 to 786b693 Compare April 8, 2022 09:53
@findepi
Copy link
Member Author

findepi commented Apr 8, 2022

( dropped stress tests )

@findepi findepi merged commit e33ebc5 into trinodb:master Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

5 participants