Propagation of column boundary changes in subexpressions #5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alamb
reviewed
Nov 15, 2022
Comment on lines
178
to
181
| let predicate_selectivity = self | ||
| .predicate | ||
| .boundaries(&analysis_ctx) | ||
| .boundaries(&mut analysis_ctx) | ||
| .and_then(|bounds| bounds.selectivity); |
There was a problem hiding this comment.
What if this looked something like:
let predicate_selectivity = self
.predicate
.boundaries(analysis_ctx)
.and_then(|analysis_ctx| analysis_ctx.bounds().selectivity);Namely that boundaries took in a context and returned a potentially modified one?
The reason I really worry about the mut analysis_ctx thing is when there is an operator that removes the intermediate column boundaries (like maybe CASE or some user defined function call) each place will have to remember to clear any intermediate boundaries built up in the context.
The idea of tracking intermediate column boundaries looks very cool
There was a problem hiding this comment.
Namely that boundaries took in a context and returned a potentially modified one?
Thinking about it, that sounds really intriguing. Will give it a try, thank you for your suggestion and feedback on this @alamb!
isidentical
force-pushed
the
gh-3845-phasse-3
branch
from
27f2ceb to
c27c321
Compare
isidentical
force-pushed
the
gh-3845-phasse-3
branch
from
c27c321 to
12d0cc6
Compare
isidentical
force-pushed
the
gh-3845-phasse-3
branch
from
a392eb8 to
12d0cc6
Compare
* Add window func related logic plan to proto ability. Signed-off-by: yangjiang <yangjiang@ebay.com> * add test. Signed-off-by: yangjiang <yangjiang@ebay.com> * more functional Signed-off-by: yangjiang <yangjiang@ebay.com> Signed-off-by: yangjiang <yangjiang@ebay.com>
Signed-off-by: yangjiang <yangjiang@ebay.com> Signed-off-by: yangjiang <yangjiang@ebay.com>
* Remove interior mutability of MemTable * remove insert_batches
* improve error handling and add some more types * refactor booleanarray
* window frame none, non empty orderby handling logic * window frame none, empty orderby handling logic * Remove window frame option * Minor changes * Use ScalarValue::Null for unbounded * Resolve proto errors * combine functions under new
* Minor: add some comments to aggregate code * Fix typos Co-authored-by: jakevin <jakevingoo@gmail.com> * fix typo Co-authored-by: jakevin <jakevingoo@gmail.com> Co-authored-by: jakevin <jakevingoo@gmail.com>
* remove project_with_alias. refactor: cleanup code of `subqueryAlias` and `expr-alias`. * separate project and subquery_alias * expr alias * replace with `.alias()` * fix comment
* minor: remove redundant `unwrap()` * more
* sqllogictest: A logging and command line filter * Reduce some println to info * Be compatible with Rust test runner * Fix typo, cargo fmt * Add a note about substring matching
…ec (apache#4508) * fix confilt Signed-off-by: yangjiang <yangjiang@ebay.com> # Conflicts: # datafusion/core/tests/sql/window.rs * fix ut Signed-off-by: yangjiang <yangjiang@ebay.com> Signed-off-by: yangjiang <yangjiang@ebay.com>
* support seconds fraction in date_part * support seconds fraction in date_part * to be insync with pgsql * fix merge * fix bench * fix bench
* Add tests for names with period * adjust docstrings * Improve docstrings * Add tests coverage * Update datafusion/common/src/table_reference.rs Co-authored-by: Nga Tran <nga-tran@live.com> * Add tests for creating tables with three periods Co-authored-by: Nga Tran <nga-tran@live.com>
* Support Time32 and Time64 for Type Coercion * Revert "Support Time32 and Time64 for Type Coercion" This reverts commit a46b97e. * Implement Time32 and Time64 in hash_join and hash_util * Add review comments * Qualify TimeUnits * Changes in proto to provide full support for Time32 and Time64 * Add test to ensure Time32 and Time64 are fully supported * Add support for type coercion for pair (Timestamp, Utf8) and add corresponding test cases * Implement Time32 and Time64 in hash_join and hash_util * Add review comments * Changes in proto to provide full support for Time32 and Time64 * Add test to ensure Time32 and Time64 are fully supported * Add support for type coercion for pair (Timestamp, Utf8) and add corresponding test cases * Revert to_proto.rs * Revert to_proto.rs * Revert timestamp.rs * Revert mod.rs * Revert select.rs * Revert group_by.rs * Revert aggregates.rs * Delete generated file
Arrow now offers `size` methods for `DataType` and `Field`, so these TODOs can be fixed.
* feat: support prepare statement * fix: typo * chore: address preliminary review comments * fix: put Placeholder last to have the expression comparison to work as expected * test: add more tests and starting to pass param_data_types to expression to get data types of the params , , ... * test: one more test and a bit of refactor while waiting for the CTEs/PlannerContext PR * feat: use prepare stmt's param data types in the planner context * chore: cleanup * refactor: address review comments * chore: cleanup * test: more prepare statement tests * chore: cleanup * chore: fix typos and add tests into the sqllogicaltests * docs: add docstring * chore: update test panic message due to recent change to have clearer message per review comment * chore: add a test and a doc string per review comments * fix: output of a test after master merge
* Add tests for coercion of timestamps to strings * Update datafusion/core/tests/sqllogictests/test_files/timestamps.slt
* MINOR: move sqllogictest to dev-dependencies * Update datafusion-cli Cargo.lock * fix toml
* extract to support functions * extract support functions
* remove Signed-off-by: remzi <13716567376yh@gmail.com> * clean Signed-off-by: remzi <13716567376yh@gmail.com> Signed-off-by: remzi <13716567376yh@gmail.com>
…e#4488) * Fix panic in median "AggregateState is not a scalar aggregate" * Apply suggestions from code review Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com> Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>
* bump sqllogictest to 0.9.0 * cargo update datafusion-cli * fix clippy * fix Cargo.lock * fix test case
…in to inner join (apache#4443) * Support non-column join key in eliminating cross join to inner join * Add comment * Make clippy happy * Add tests * Add alias for cast expr join keys * Add tests * Add relative issue comment * Improve test * Improve use declarations
* Port create_drop.rs tests to sqllogictest framework * Add in expected error messages
isidentical
force-pushed
the
gh-3845-phasse-3
branch
from
12d0cc6 to
629c6e4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proof of concept.