Use standard TPCH configs in development server #17321
Conversation
|
CC: @findepi |
|
This change fine with me, but I remember there was some counter argument from changing it by default though I don't remember what exactly it was |
| connector.name=tpch | ||
| tpch.splits-per-node=4 | ||
| tpch.column-naming=STANDARD | ||
| tpch.double-type-mapping=DECIMAL |
There was a problem hiding this comment.
Define "standard".
IMO this file should have only connector.name=tpch and everything beyond that is redundant
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
I haven't changed defaults, just the config for the dev server catalog |
3 participants
Description
Additional context and related issues
Release notes
(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: