Skip to content

Use standard TPCH configs in development server #17321

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.

Sign up for GitHub

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

Merged
raunaqmorarka merged 1 commit into from
May 3, 2023
Merged

Use standard TPCH configs in development server #17321

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions testing/trino-server-dev/etc/catalog/tpch.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@

connector.name=tpch
tpch.splits-per-node=4
tpch.column-naming=STANDARD
tpch.double-type-mapping=DECIMAL
Copy link
Member

@findepi findepi May 2, 2023

Choose a reason for hiding this comment

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

Define "standard".

IMO this file should have only connector.name=tpch and everything beyond that is redundant

Copy link
Member Author

@raunaqmorarka raunaqmorarka May 2, 2023

Choose a reason for hiding this comment

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

Standard is defined at https://www.tpc.org/tpc_documents_current_versions/pdf/tpc-h_v3.0.1.pdf
The naming convention and usage of decimal type is specified by the TPCH standard. This is already what we benchmark, I'm just making the dev setup consistent with what we benchmark so that devs don't have to keep filling this in by hand when working with files locally.

Copy link
Member

@findepi findepi May 4, 2023

Choose a reason for hiding this comment

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

Standard is defined at https://www.tpc.org/tpc_documents_current_versions/pdf/tpc-h_v3.0.1.pdf
The naming convention and usage of decimal type is specified by the TPCH standard.

Sure, let's fix the tpch connector to follow the standard

This is already what we benchmark, I'm just making the dev setup consistent with what we benchmark

that's convenient from benchmark perspective, but that's not convenient from integration tests perspective
the deve server should be consistent with both, so it serves developers well

I'm just making the dev setup consistent with what we benchmark so that devs don't have to keep filling this in by hand when working with files locally.

people may be firing queries taken from integration tests and these won't work anymore

Copy link
Member Author

@raunaqmorarka raunaqmorarka May 4, 2023

Choose a reason for hiding this comment

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

Valid point about integration tests. I care more about the using decimal types than matching the naming convention, so created #17344 to hopefully find a better middle ground.