feat(data-warehouse): Override external data table fields

[Gilbert09]

Problem

• We want to be able to change the data structure of external data tables without actually modifying the underlying data. This is so that we can do some table modeling and clean-up, such as parsing Unix timestamps into a DateTime type

Changes

• Allows the hogql fields of external data tables to be overridden in code
• Adds functionality to "hide" database fields in hogql:
  • This removes the field from autocomplete, and
  • removes the field from getting returned via *
  • But, the field can still be queried directly if need be, we just don't expose the names
• Also adds the ability for expression fields to get returned from * queries (effectively reverting feat(query-engine): Remove expression fields from asterisk expansion #19239)
• Adds some overrides for the stripe_customer table as an example
  • This parses the created field from stripe (a unix timestamp) into created_at (a DateTime clickhouse type)

How did you test this code?

• Unit tests

[mariusandra]

Had a look, and seems good. I'm not sure about one part through (replied inline).

Also, we now have hidden aliases and hidden fields, where the meaning of "hidden" is slightly different. I wonder if we should disambiguate? 🤔 🤷

[mariusandra]

Since I can imagine users having some of their own tables with __-prefixed fields, and someone someday having some conflict, if we expand this further or make it automatic. Should we be even harsher with something like:

        "__x_created": IntegerDatabaseField(name="created", hidden=True),

No strong feelings, but thought it worth mentioning.

[mariusandra]

I wonder if this is correct. In the resolver, when we resolve expression fields, we clone this node.expr and swap out the node with ExpressionFieldType with the cloned expr.

The change here probably has some side effects, and/or might mutate the "reference expression" instead of cloning and applying. Not sure, but it might be worth double checking.

[Gilbert09]

Interesting, yeah, some tests were failing due to TraversingVisitor not having a visit_expression_field_type func - we do pass on some other _type funcs, which may be better here than visiting the expression

[mariusandra]

Looks good! 👍

The only thing you should keep in mind is that the expression fields aren't perfect (finished). The code that swaps the field out with the expression does a very simple clone_expr operation. This means that under certain conditions, the expression field will "resolve" to garbage.

For example this nonsense query:

select * 
from stripe_invoices a 
left join stripe_invoices b 
on a.amount_paid = b.amount_paid
where a.period_start_at < b.period_start_at

will be swapped with:

select * 
from stripe_invoices a 
left join stripe_invoices b 
on a.amount_paid = b.amount_paid
where __period_start < __period_start

I think to solve it, we should update the cloning code here, and push a fake ast.SelectQueryType onto self.scopes (with just that one table) before visiting and pop it after. That should give everything inside the expression field the right type, and they should get the a.__period_start added automatically.