Skip to content

Aggregate syntax transform #24

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

Open
wants to merge 3 commits into
base: REL_11_CARTO
Choose a base branch
from

Conversation

rafatower
Copy link

No description provided.

Rafa de la Torre added 3 commits August 2, 2019 15:50
What this does:

- it adds a ton of traces, that are more suitable to follow certain
  aspects of the code that raw gdb on the foreign_expr_walker
  recursive exploration of the parse tree.

- it modifies the code related to checking of collations. I don't
  really understand if it is a bug or if there's actually a conflict
  with collations and it is unsafe to push, but I tend to think it is
  indeed a bug as no collation should affect it.

- 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 really valid and fails
  with an error on the remote end. It shall not confuse the
  representation in the EXPLAIN of ROW() with the SQL ROW() row
  constructor.

In short this just half-works, and we really need both halfs for it to
do something interesting. Nevertheless I think it is good to share, to
give an idea of the type of work to accomplish to finish this and
similar cases.
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant