add subset dataset facets to spec by pawel-big-lebowski · Pull Request #4008 · OpenLineage/OpenLineage

pawel-big-lebowski · 2025-09-02T11:50:56Z

Part of the [Spark] Normalize dataset names with configurable trimmers #3996 with spec changes only.
This is easier to review.
Rest of the [Spark] Normalize dataset names with configurable trimmers #3996 will be a follow-up PR.
Java code generator changes are added to support const field in Json-Schema

codecov-commenter · 2025-09-02T12:34:45Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.05%. Comparing base (d10b39f) to head (6593b26).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4008   +/-   ##
=======================================
  Coverage   85.05%   85.05%           
=======================================
  Files          57       57           
  Lines        3855     3855           
=======================================
  Hits         3279     3279           
  Misses        576      576

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull Request Overview

This PR adds subset dataset facets to the OpenLineage specification, introducing a new schema definition for filtering or subsetting datasets based on various conditions like location, partition, comparison, or binary operations.

Introduces a new BaseSubsetDatasetFacet.json schema with comprehensive condition types for dataset subsetting
Adds test cases demonstrating different subset condition scenarios (compare, binary, location, partition)
Updates Java code generation to support const fields in JSON Schema

Reviewed Changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
website/static/spec/facets/1-0-0/BaseSubsetDatasetFacet.json	Main schema definition for subset dataset facets with various condition types
spec/facets/BaseSubsetDatasetFacet.json	Duplicate schema file in spec directory
spec/tests/BaseSubsetDatasetFacet/*.json	Test cases for different subset condition scenarios
client/python/redact_fields.yml	Python client configuration for new subset dataset classes
client/python/openlineage/client/facet_v2.py	Python client import for new subset dataset module
client/java/src/test/java/io/openlineage/client/OpenLineageTest.java	Java test demonstrating subset facet usage
client/java/generator/src/main/java/io/openlineage/client/*.java	Java code generator updates to support const fields

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

client/python/redact_fields.yml

julienledem

That makes sense, I have added comments regarding the code generator portion.

julienledem · 2025-09-03T21:53:37Z

client/java/generator/src/main/java/io/openlineage/client/SchemaParser.java

+              if (field.getValue().has("const")) {
+                // only String const are supported
+                fields.add(new Field(field.getKey(), new PrimitiveType("string", null), description, field.getValue().get("const").asText()));
+              } else {


shouldn't this logic be in the parse() method instead?

I think not, bcz const shouldn't be returned as a Type (like PrimitiveType of String with constant value) and it should be already put at the Field member.

julienledem · 2025-09-03T21:55:50Z

client/java/generator/src/main/java/io/openlineage/client/SchemaParser.java

+    public PrimitiveType(String name, String format, String constValue) {
+      super();
+      this.name = name;
+      this.format = format;
+    }


constValue is not used?

Thx, constValue should be just a property of a Field instead.

julienledem · 2025-09-03T22:02:57Z

client/java/generator/src/main/java/io/openlineage/client/TypeResolver.java

+            if (property.getConstValue().isPresent()) {
+              resolvedField = new ResolvedField(property, visit(property.getType()), property.getConstValue().get());
+            } else {
+              resolvedField = new ResolvedField(property, visit(property.getType()));
+            }


This seems unnecessarily complicated. You are unwrapping an Optional to pass it to a constructor that will rewrap it in an Optional.
Maybe just add a ResolvedField constructor that takes an optional since this is how it stores it.
ResolveField(Field field, ResolvedType type, Optional<String> constantValue)

julienledem · 2025-09-03T22:03:26Z

client/java/generator/src/main/java/io/openlineage/client/TypeResolver.java

+
    @Override
    public String toString() {
      return "ResolvedField{name: " + field.getName() + ", type: " + type +  "}";


should be updated.

same for equals and hashCode bellow

Changing the code so that constantValue is stored directly within field, so there's no need to change toString, equals, and hashCode for ResolvedField. Thanks.

julienledem · 2025-09-05T23:06:43Z

client/java/generator/src/main/java/io/openlineage/client/SchemaParser.java

-              fields.add(new Field(field.getKey(), fieldType, description));
+              if (field.getValue().has("const")) {
+                // only String const are supported
+                fields.add(new Field(field.getKey(), new PrimitiveType("string", null), description, field.getValue().get("const").asText()));


to avoid surprises maybe we should still call parse() but check the type is String and throw an exception if not.
With this implementation, one can put whatever in the type and it will be replaced by string.
This could lead to some hair pulling on unexpected behavior.

Makes sense, we should definitely avoid hair pulling. Second commit should resolve that.

mobuchowski

Approved but would appreciate more comments around the Java generator part - it's less frequently accessed and looked at part of code, so it's a good opportunity to document it more. Thanks.

julienledem · 2025-09-10T17:21:08Z

client/java/generator/src/main/java/io/openlineage/client/JavaPoetGenerator.java

      // only regular fields are used in the builder
      @Override
      public void onField(ResolvedField f) {
+        if (f.getConstantValue().isPresent()) {


instead of adding this if statement in every call to onField(), you could add an onConstantField() to the Visitor with the default implementation that does nothing.

Love this—this was exactly what I was missing. Thanks!

julienledem

I made an additional comment that can simplify a bit the section about handling constant fields.
Otherwise, this looks good.

Signed-off-by: Pawel Leszczynski <leszczynski.pawel@gmail.com>

pawel-big-lebowski requested a review from a team as a code owner September 2, 2025 11:50

boring-cyborg bot added area:client/java openlineage-java area:client/python openlineage-python area:spec Specifications and standards for the project area:tests Testing code language:java Uses Java programming language language:python Uses Python programming language labels Sep 2, 2025

pawel-big-lebowski marked this pull request as draft September 2, 2025 11:52

pawel-big-lebowski force-pushed the spec/subset-definition-facet branch 2 times, most recently from fdf951d to 18aed40 Compare September 2, 2025 11:58

pawel-big-lebowski marked this pull request as ready for review September 2, 2025 12:31

JDarDagran requested a review from Copilot September 2, 2025 13:45

This comment was marked as outdated.

Sign in to view

pawel-big-lebowski requested a review from julienledem September 2, 2025 15:10

JDarDagran requested a review from Copilot September 2, 2025 16:29

Copilot AI reviewed Sep 2, 2025

View reviewed changes

client/python/redact_fields.yml Show resolved Hide resolved

julienledem reviewed Sep 3, 2025

View reviewed changes

pawel-big-lebowski force-pushed the spec/subset-definition-facet branch 2 times, most recently from 5347ec7 to 6593b26 Compare September 4, 2025 11:50

julienledem reviewed Sep 5, 2025

View reviewed changes

pawel-big-lebowski force-pushed the spec/subset-definition-facet branch from 6593b26 to 3eed2aa Compare September 8, 2025 09:37

mobuchowski approved these changes Sep 10, 2025

View reviewed changes

julienledem reviewed Sep 10, 2025

View reviewed changes

julienledem approved these changes Sep 10, 2025

View reviewed changes

pawel-big-lebowski force-pushed the spec/subset-definition-facet branch from 3eed2aa to 9266d7a Compare September 11, 2025 16:02

pawel-big-lebowski added 3 commits September 12, 2025 09:23

add subset dataset facets to spec

9175e3a

Signed-off-by: Pawel Leszczynski <leszczynski.pawel@gmail.com>

throw error for non-string const type

a580670

Signed-off-by: Pawel Leszczynski <leszczynski.pawel@gmail.com>

extend ResolvedFieldHandler with onConstantField

9623d1e

Signed-off-by: Pawel Leszczynski <leszczynski.pawel@gmail.com>

pawel-big-lebowski force-pushed the spec/subset-definition-facet branch from 9266d7a to 9623d1e Compare September 12, 2025 07:23

pawel-big-lebowski merged commit d0944e9 into main Sep 12, 2025
67 checks passed

pawel-big-lebowski deleted the spec/subset-definition-facet branch September 12, 2025 15:29

Conversation

pawel-big-lebowski commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

julienledem left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pawel-big-lebowski Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mobuchowski left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julienledem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pawel-big-lebowski commented Sep 2, 2025 •

edited

Loading

codecov-commenter commented Sep 2, 2025 •

edited

Loading

pawel-big-lebowski Sep 4, 2025 •

edited

Loading