Repository navigation
feat(core-spec): add table sources with optional identifier levels - #704
johndacosta wants to merge 17 commits into
Conversation
Proposal for the dev@ thread "[DISCUSS][core-spec] What should Dataset.source guarantee?". Not for merge before lazy consensus. Dataset.source keeps its string form with the same meaning. It also accepts an object with a kind: table identifier of one table or view query dialects list, same shape as expression dialects file format and locations The spec says what a consumer may assume for each kind and when it may reject one (GitHub issue apache#298). The kind field follows apache#173 (d-ralhan); identifier, format and locations follow apache#338 (isaac-dasari). Dataset.filter is an optional boolean expression in the expression language. Consumers apply it as WHERE on the source everywhere the dataset is used. expression_language.md already pointed WHERE at a "filter property"; this adds it. The filter is separable from the source change. Adds examples/dataset_sources.yaml, schema and model support, filter SQL checks in validate.py, and tests. Generated-by: Grok Bot
…inds-and-filter # Conflicts: # validation/validate.py
…ters A dataset filter is applied to each source row before any aggregation, so it must be a row-level predicate. The validator now parses every dataset_filter expression (ANSI and dialect entries) with sqlglot and reports an error when the tree contains an aggregate function (exp.AggFunc, including COUNT(*)), a window function (exp.Window), or the spec's AGG() wrapper. Tests cover SUM, COUNT(*), AVG, AGG, ROW_NUMBER() OVER, SUM() OVER, a dialect-specific entry, accepted row-level predicates, and that metrics and fields are not checked by this rule. Generated-by: Grok Bot
The dev@ thread agreed to settle portable table references first and to discuss filters separately. This removes Dataset.filter, its validation rule and tests from this branch. The work is kept unchanged on feat/dataset-filter (e4aa3c1). Generated-by: Grok Bot
A dotted string cannot say where one name part ends: Iceberg namespaces nest, and Polaris allows dots in table names, so converters split the same string differently. A table source's identifier is now a list of parts, outermost first, each the exact stored name. A string source that names a table stays valid as shorthand, split on every ".". The spec also says how a table is resolved: each consumer matches the leading parts against its own connections and reports no match or more than one rather than picking. Connection details stay out of the identifier and go in custom_extensions. A consumer rejects a kind it does not support. Generated-by: Grok Bot
…fiers Chris asked to settle portable table references end to end before generalizing, so `kind` now defines only `table`. The query and file kinds are kept unchanged on feat/dataset-source-query-file (c8b7abc). - Unknown kinds fail validation by name ('view' is not one of ['table']). - Identifier parts must be strings, not blank, and free of control characters and U+2028/U+2029. - String shorthand is defined: parts joined by single dots, with no empty part, whitespace, quote, bracket, "(", ")", ";" or ",". Anything else, such as a query or "db..t", stays valid but is not split; the validator warns. The pattern runs in linear time. - ossie.table_identifier() returns the parts of a table source or shorthand string, or None. - The spec adds that consumers quote parts as delimited identifiers, never open a connection a document names, and that identifiers and custom_extensions carry no secrets. Generated-by: Grok Bot
Keeps upstream's OssieVendor import alongside this branch's table source imports in python/tests/test_models.py. Generated-by: Grok Bot (ossiebot)
Keep both sides: the dataset sources example step and tests sit next to upstream's renamed-fields step and Name Resolution tests. Generated-by: Grok Bot (ossiebot)
A table source may say whether its identifier is complete. relative, the default, lets the consumer supply leading parts from the connection the user chose; the consumer decides how many. absolute means the name is complete; a one-part absolute identifier is invalid. A consumer that cannot supply the missing parts, or cannot represent the part count, rejects the source and names the parts it needs, instead of writing an empty name. The flights examples drop the DATABASE.SCHEMA placeholders and use relative names. Generated-by: Grok Bot
# Conflicts: # validation/tests/test_validate.py
Drop completeness qualification. Keep exact ordered names unrestricted in number, with no level inferred for plain strings. Producers may label any part, while consumers bind leading context or reject clearly instead of flattening or inventing names. Generated-by: Grok Bot
spec.yaml types source as a string or a table-source object, with one identifier per example and no positional level names. Non-shorthand warnings say to use kind: table only when the string names a table, and query strings stay valid. Flights examples keep their DATABASE.SCHEMA names. Schema and Python tests reject U+0001, U+0085, U+2028, and blank identifier parts. Generated-by: Grok Bot
The new validation step reads examples/dataset_sources.yaml, but the workflow path filters did not, so an example-only change skipped CI. Add that file to the push and pull_request filters. Generated-by: Grok Bot
kayemkim
left a comment
There was a problem hiding this comment.
Two sentences disagree on the same case: under Source a consumer MAY reject a source whose identifier it cannot resolve, under Resolving a Table it MUST reject it with a diagnostic. The MUST is what the thread settled on, so the first bullet could defer to it.
The example and the Source snippet carry the connection as custom_extensions with data as a JSON string. #522 renames the key to nonportable_extensions and makes data an object, and on the merged tree examples/dataset_sources.yaml fails the new CI step with 'custom_extensions' was unexpected. Whichever lands second takes the rename and data: {connection: warehouse_prod}.
#698 adds a validate_dataset_sources of its own that parses a string source starting with SELECT or WITH, so the two need merging by hand, and a query source would then get this [Source] warning next to #698's parse result. The existing examples all use shorthand and validate exactly as on main, and the new example round-trips through the SDK unchanged.
The Source section said a consumer MAY reject an identifier it cannot resolve, while Resolving a Table says it MUST reject that case with a diagnostic. Point the Source bullet at that rule and test the two sections stay consistent. Generated-by: Grok Bot
…filter Resolved the sole conflict in core-spec/spec.md Version History by keeping apache#691's 0.3.0.dev0 heading and replacement note, and re-applying apache#704's Dataset.source table-identifier changelog bullet. Bumped examples/dataset_sources.yaml from 0.2.0.dev0 to 0.3.0.dev0 to match main.
|
We are a producer that needs both of the shapes this PR allows, so here ElastiCORE (XSOLCORP KOREA) generates a Spring application and its When the model names a schema, we know the name and we know it is a So one producer writes both: identifier:
- {name: acct, level: schema}
- {name: customer, level: table}
identifier: [ps_category]We changed our exporter yesterday so that it stops dropping the schema: For now it writes the dotted string, Our sample project still exports one part only, because that model names On the open checklist item: ossie-converters still assumes positional (Same producer as the dev@ thread; I sent the mail as Richard Kim.) |
|
Hey all, chiming in from the OpenLineage project. It's my first time engaging with the Ossie community :) +1 to the list form with optional levels. It also matches how we treat the other part of the identifier: like in this PR, we don't treat the system/instance as part of the name. The proposal of a binding separate from the model seems like the right place for that, and we'd be glad to collaborate on the naming conventions there if it's useful. Looking forward to start the collaboration, didn't necessarily want to take over this PR discussion, but felt like a good place to start? should I be posting this to the mailing list instead? |
# Conflicts: # core-spec/spec.md
Summary
Adds table sources with ordered, exact identifier parts and optional {name, level} labels. Plain parts imply no level, and the number of parts is unrestricted. Consumers supply missing leading context or reject clearly rather than flattening names.
Existing string sources, including queries, remain unchanged. Query objects and per-dialect sources are outside this PR. No breaking change to the released spec; qualification existed only in unreleased branch work.
Discussed on dev@: https://lists.apache.org/thread/zkxh5k42l39bg0clj6mff2mb1h12ml5o
Related to #298 and #698.
Verification
At commit 632b63b:
Downstream compatibility
apache/ossie-converters still requires string sources and assumes positional identifier parts. Table objects and arbitrary part counts need a separate converter change; a matching issue has not yet been filed. Existing three-part strings remain supported.
Checklist
Prepared with Grok Bot. Feature and correction commits include Generated-by: Grok Bot. Licensed under Apache-2.0.