rdblue commented on code in PR #7504: URL: https://github.com/apache/iceberg/pull/7504#discussion_r1199813515
########## format/view-spec.md: ########## @@ -111,12 +111,13 @@ The SQL representation stores the view definition as a SQL SELECT, with metadata |-------------|---------------------|----------------|-------------| | _required_ | `type` | `string` | Must be `sql` | | _required_ | `sql` | `string` | A SQL SELECT statement | -| _required_ | `dialect` | `string` | The dialect of the `sql` SELECT statement (e.g., "trino" or "spark") | +| _required_ | `dialect` | `string` | The dialect of the `sql` SELECT statement (e.g., "trino" or "spark"). Unless a version is provided only one view definition for each `type` and `dialect` should be present in a view definition. | Review Comment: > I thought the intent was to support multiple dialects, for the same type, but maybe the intent here is to say "type" is the unique key here? Which raises the question should dialect be required? It seems like a nice to have for compatibility reasons? The `type` is basically how we are identifying the representation. Representations are like a union of different concrete representations and `type` selects it. Then `dialect` is specific to SQL. In another PR, we clarified that you can have multiple `sql` representations with different dialects. I think you always want to have a dialect to give some indication of what to expect, and so you can map dialects to different handling when using a view definition. > I think the use-case here would be migrations for the cases where changes are breaking? I'm skeptical we would do this automatically. SQL is not going to have very good compatibility. While we can help that out a little and should track dialect, the best solution will often be to either use a more strict representation (like a future IR one) or to replace the view with a new version that works. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
