-
Notifications
You must be signed in to change notification settings - Fork 43
Use maybe constant in duckdb convert #3258
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
Conversation
Signed-off-by: Joe Isaacs <[email protected]>
Signed-off-by: Joe Isaacs <[email protected]>
Benchmarks: TPC-H on NVMETable of Results
|
Benchmarks: TPC-H on S3Table of Results
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to request changes just to block this as an experiment-only. I think the fix looks more like: #3274
wait, this is on of the options mentioned in the issue. Why is asking for an existing stat an antipattern now? If it is we should remove this api? |
It's an anti-pattern because this call site wants an estimate of "is_constant" with some predicatable performance guarantees. Query the stats fits the performance requirement, but there are other sources that also may fit the requirement. e.g. ConstantArray that doesn't pre-populate the stats. Or dictionary array with constant codes. So we should add an API that lets this and other call sites express "what" they want, not "how" they want it |
Fixed by #3283 |
No description provided.