[DNM] Aggregate syntax transform cleanup #25
Conversation
This file contains hidden or 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
rafatower
force-pushed
the
aggregate-syntax-transform-cleanup
branch
from
126b061 to
b6e44e2
Compare
What this does: - it allows for Row Expressions node push downs (T_RowExpr) in a very hacky way. Chances are that to do it properly it needs to continue with a recursive exploration of the tree from that node. This was preventing the whole aggregate function ST_AsMVt to be pushed down to the foreign server. - it provides a crappy implementation of deparseRowExpr. It makes the planner happy to ship some SQL but it is not 100% valid as column names are lost when received in the foreign end.
rafatower
force-pushed
the
aggregate-syntax-transform-cleanup
branch
from
b6e44e2 to
7ee44e1
Compare
rafatower
force-pushed
the
aggregate-syntax-transform-cleanup
branch
from
7ee44e1 to
558f344
Compare
added 3 commits
September 16, 2019 13:21
For default input collation, inner_cxt.state is marked as FDW_COLLATE_NONE. There's the situation in which agg.collation == DEFAULT_COLLATION_OID and thus it should be safe to push down the aggregation.
A row expression cannot be deparsed as ROW(...) cause otherwise column names are lost when sending the deparsed query to the remote. RowExpr may be present in the planner tree, as the optimizer may "pull up" aggregation subqueries and it certainly does. In particular, ST_AsMVT may produce different results or may miss the geom column if column names are generated as f1, f2, ..., fn. This solves that issue by deparsing the row expression as an alias and embedding it into a subquery. This is the intended syntax transformation in pseudo-code: SELECT row_expr FROM from_clause WHERE where_caluse => SELECT alias FROM ( SELECT row_expr_fields FROM from_clause WHERE where_clause ) alias
That is the schema where postgis (and many other extensions) usually reside. This is done in order to overcome a bug in postgis type checking that yields "ERROR: parse_column_keys: no geometry column found". That bug was recently fixed in this commit: postgis/postgis@e096795#diff-86bc92dfea8fd30b975e278bd323067f
rafatower
force-pushed
the
aggregate-syntax-transform-cleanup
branch
from
558f344 to
6fe4734
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.
I plan to rebase, add proper commit messages and remove intermediate steps