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

Make sure that test queries process less input bytes with work processor pipelines #1253

Merged
merged 5 commits into from Aug 6, 2019

Conversation

@sopel39
Copy link
Member

commented Aug 4, 2019

No description provided.

@sopel39 sopel39 requested a review from Praveen2112 Aug 4, 2019

@cla-bot cla-bot bot added the cla-signed label Aug 4, 2019

@sopel39 sopel39 requested a review from dain Aug 4, 2019

@sopel39 sopel39 force-pushed the starburstdata:ks/work_processor_tests branch 2 times, most recently from 347527a to deb13df Aug 4, 2019

@dain

dain approved these changes Aug 5, 2019

Copy link
Member

left a comment

Some suggestions, but nothing super important.


public class TpchConnectorFactory
implements ConnectorFactory
{
public static final String TPCH_COLUMN_NAMING_PROPERTY = "tpch.column-naming";
public static final String TPCH_MAX_ROWS_PER_PAGE_PROPERTY = "tpch.max-rows-per-page";
private static final int DEFAULT_MAX_ROWS_PER_PAGE = 1000000;

This comment has been minimized.

Copy link
@dain

dain Aug 5, 2019

Member

Add underscores to the number

{
return new TpchRecordSetProvider();
return new TpchPageSourceProvider(getMaxRowsPerPage(properties));

This comment has been minimized.

Copy link
@dain

dain Aug 5, 2019

Member

I'm thinking we might want to make this a configuration option. In the old code it tested the connector record set code that has a special test for RecordPageSource in Presto main that trigger different behavior.

This comment has been minimized.

Copy link
@sopel39

sopel39 Aug 6, 2019

Author Member

I will make record set the default behavior

@@ -46,6 +46,7 @@ protected TestWorkProcessorPipelineQueries()
public void testTopN()
{
assertLazyQuery("SELECT * FROM orders ORDER BY totalprice LIMIT 10");
assertLazyQuery("SELECT * FROM orders WHERE orderkey >= 0 ORDER BY totalprice LIMIT 10");

This comment has been minimized.

Copy link
@dain

dain Aug 5, 2019

Member

IIRC all order keys are positive, so one day this filter could be eliminated by table constraints. Maybe pick a value like 10?

@dain dain assigned sopel39 and unassigned dain Aug 5, 2019

@sopel39 sopel39 force-pushed the starburstdata:ks/work_processor_tests branch from deb13df to 4949c8f Aug 6, 2019

@sopel39 sopel39 merged commit 51131ff into prestosql:master Aug 6, 2019

1 of 2 checks passed

Travis CI - Pull Request Build Errored
Details
verification/cla-signed
Details

@sopel39 sopel39 deleted the starburstdata:ks/work_processor_tests branch Aug 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.