Change syntax for constructing a record with a single field#4295
Change syntax for constructing a record with a single field#4295hazefully wants to merge 7 commits into
Conversation
This commit changes the syntax for constructring an anonymous record with a single field from `(..)` to `STRUCT (..)`. The reason for this change is to remove the need for the automated flattening of function arguments with single item records which happens in some contexts.
📊 Metrics Diff Analysis ReportSummary
ℹ️ About this analysisThis automated analysis compares query planner metrics between the base branch and this PR. It categorizes changes into:
The last category in particular may indicate planner regressions that should be investigated. New QueriesCount of new queries by file:
Dropped QueriesThe following queries with metrics were removed:
The reviewer should double check that these queries were removed intentionally to avoid a loss of coverage. Plan and Metrics ChangedThese queries experienced both plan and metrics changes. This generally indicates that there was some planner change Total: 1 query Statistical Summary (Plan and Metrics Changed)
Significant Regressions (Plan and Metrics Changed)There was 1 outlier detected. Outlier queries have a significant regression in at least one field. Statistically, this represents either an increase of more than two standard deviations above the mean or a large absolute increase (e.g., 100).
|
| import java.util.Set; | ||
| import java.util.function.BiFunction; | ||
|
|
||
| public final class VariantCommand extends Command { |
There was a problem hiding this comment.
Can you document this please?
There was a problem hiding this comment.
I am not sure I am for the introduction of another YAML command currentVersionAtLeast, it looks very similar to initialVersionAtLeast. and it seems the differences between the two are purely technical, can we unify these two? if there are differences related to handling continuations across different server versions, then that seems like an internal testing framework detail that shouldn't be exposed at this level.
|
|
||
| @Nonnull | ||
| private final YamlReference reference; | ||
| final YamlReference reference; |
There was a problem hiding this comment.
It is probably a better idea to make this protected instead of using package-level visibility.
| # contain a flat record with the scalar value. | ||
| - query: SELECT "b".* FROM (SELECT "a" FROM VALUES ([1, 2, 3, 4]) AS T("a")) AS "sq", (SELECT ("x") AS wrapped FROM "sq"."a" AS "x") AS "b" | ||
| - initialVersionLessThan: !current_version | ||
| - result: [{wrapped: {1}}, {wrapped: {2}}, {wrapped: {3}}, {wrapped: {4}}] |
There was a problem hiding this comment.
This breaks continuations, so we have to be careful with the rollout.
| # Double-wrapped (*) nesting is unaffected for older versions | ||
| - query: select ((*)) from t1 where pk = 1 |
There was a problem hiding this comment.
# Double-wrapped (*) nesting is unaffected for older versions
... for versions older than !current_version, I think.
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
|
||
| /** | ||
| * Tests that tests with setup blocks based on the current version are executed correctly. |
| : '(' expressionWithOptionalName (',' expressionWithOptionalName)* ')' | ||
| ; | ||
|
|
||
| expressionWithPrecedence |
There was a problem hiding this comment.
Can you rename this to expressionWithParenthesis? defining "precedence" at this high level is a bit too strong of a rule.
|
|
||
| expressionAtom | ||
| : constant #constantExpressionAtom // done | ||
| : expressionWithPrecedence #expressionWithPrecedenceAtom // done |
There was a problem hiding this comment.
Also here, please rename the rule branch to expressionWithParethesisAtom
| Struct Type Declaration | ||
| ======================= | ||
|
|
||
| You can define a *struct type* (often interchangeably referred to as a *nested type*). A struct is a tuple of columns that allow the same types as a table does, but does _not_ have a primary key. Struct types are "nested" within another owning type, and are stored in the same location as their owning record. For example, a table :sql:`foo` can have the following layout (using the :doc:`DDL <sql_commands/DDL>` syntax): |
There was a problem hiding this comment.
You can define a struct type (often interchangeably referred to as a nested type). A struct is a tuple of columns that allow the same types as a table does, but does not have a primary key. Struct types are "nested" within another owning type, and are stored in the same location as their owning record. For example, a table :sql:
foocan have the following layout (using the :doc:DDL <sql_commands/DDL>syntax):
While you're at it, here is a suggested more concise rewording:
A struct is defined almost like a table, consisting of a tuple of attributes but lacking a primary key. Structs can be nested, as demonstrated in the following example definition:
| Struct Literals | ||
| =============== | ||
|
|
||
| A *struct literal* constructs a struct value inline, directly within a query, instead of reading it from a table. Struct literals are accepted anywhere a value is expected, most commonly in :sql:`INSERT` statements, :sql:`SELECT` projections, and :sql:`WHERE` comparisons. |
There was a problem hiding this comment.
A struct literal constructs a struct value inline, directly within a query, instead of reading it from a table
suggestion:
a struct literal is an expression, it instantiates a struct expression in place. It is commonly used in ....
| This returns a single column whose value is a :sql:`STRUCT` containing one field, which is itself the row :sql:`STRUCT`. | ||
|
|
||
| .. note:: | ||
| The :sql:`STRUCT` keyword is required here since we are constructing a struct literal with a single field which is the row struct, see :ref:`struct_types` for more details. |
Note to reviewer: it might be better to review ebfd935 independently first, since it is isolated from the main change in this PR but it is required to make mixed version testing possible.
This PR changes the syntax for constructing an anonymous struct/record with a single field from
(..)toSTRUCT (..). The reason for this change is to remove the need for the automated flattening of function arguments with single item records which used to happen in some contexts while parsing the query, and would lead to issues when we have more user-defined table/scalar functions that accept user-defined struct types.After this PR, the new syntax for constructing an anonymous record with multiple fields will be
(field1, field2, ...)or the equivalentSTRUCT (field1, field2, ...). To construct a record with a single field, the syntax will beSTRUCT (field1). The existing syntax(<expr>)will be interpreted as an expression that has a higher precedence in evaluation over surrounding expressions.To make testing the breaking change in this PR possible in YAML tests, the commit ebfd935 extends the syntax introduced in #4155 to support multiple variants for
schema_templateblocks in YAML tests to support multiple variant for a single step insetupblock as well, making it possible to have different queries executed based on the current version under test that would run the setup step. The syntax for the multi-variant command block is changed to reflect the fact that we want the YAML tests framework to choose which variant to run based on the current version that would run the command, instead of the initial version. The syntax for a multi-variant schema template is now:Similarly, a step in a setup block can have multiple variants: