You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Prathamesh9284 (via GitHub)" <gi...@apache.org> on 2026/02/10 09:24:38 UTC

[PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Prathamesh9284 opened a new pull request, #1376:
URL: https://github.com/apache/datafusion-python/pull/1376

   # Which issue does this PR close?
   
   Closes #508 
   
    # Rationale for this change
   
   Other Substrait producers (Isthmus, DuckDB) support generating JSON-formatted Substrait plans, which makes it much easier to inspect and compare plans across engines. The substrait crate already provides serde support via the pbjson feature, so DataFusion Python can leverage this to expose JSON plan generation and parsing with minimal effort.
   
   # What changes are included in this PR?
   
   - **`python/datafusion/substrait.py`**
     - Added `to_json()` instance method and `from_json()` static method on the `Plan` class for converting plans to/from JSON strings.
     - Added `serialize_json()` and `deserialize_json()` static methods on the `Serde` class for writing/reading JSON plan files.
   
   - **`src/substrait.rs`**
     - Implemented the Rust backend for the above Python methods, including error mapping.
     - Added the following functions:
       - `fn to_json(&self) -> PyDataFusionResult<String>`
       - `#[staticmethod] fn from_json(json: &str) -> PyDataFusionResult<PyPlan>`
       - `#[staticmethod] pub fn serialize_json(sql: &str, ctx: PySessionContext, path: &str, py: Python) -> PyDataFusionResult<()>`
       - `#[staticmethod] pub fn deserialize_json(path: &str) -> PyDataFusionResult<PyPlan>`
   
   - **`src/errors.rs`**
     - Added `SerdeJsonError` and `DecodeError` error variants to `PyDataFusionError` for proper error handling.
   
   - **`Cargo.toml`**
     - Added `serde_json` as a dependency.
   
   - **`python/tests/test_substrait.py`**
     - Added tests for both in-memory and file-based JSON plan round-trips.
   
   # Are there any user-facing changes?
   
   Yes — four new public API methods:
   
   - `Plan.to_json() -> str` — convert a Substrait plan to a JSON string  
   - `Plan.from_json(json: str) -> Plan` — create a Substrait plan from a JSON string  
   - `Serde.serialize_json(sql, ctx, path)` — generate a JSON-formatted Substrait plan file from a SQL query  
   - `Serde.deserialize_json(path) -> Plan` — read a Substrait plan from a JSON file
   
   <!--
   If there are any breaking changes to public APIs, please add the `api change` label.
   -->


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer merged PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "Prathamesh9284 (via GitHub)" <gi...@apache.org>.
Prathamesh9284 commented on code in PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#discussion_r2797728018


##########
python/tests/test_substrait.py:
##########
@@ -74,3 +74,48 @@ def test_substrait_file_serialization(ctx, tmp_path, path_to_str):
     expected_actual_plan = ss.Consumer.from_substrait_plan(ctx, actual_plan)
 
     assert str(expected_logical_plan) == str(expected_actual_plan)
+
+def test_substrait_plan_to_from_json(ctx):
+    # Create a simple plan
+    sql = "SELECT * FROM t"
+    batch = pa.RecordBatch.from_arrays(
+        [pa.array([1, 2, 3]), pa.array([4, 5, 6])],
+        names=["a", "b"],
+    )
+    ctx.register_record_batches("t", [[batch]])
+
+    # Serialize to plan
+    plan = ss.Serde.serialize_to_plan(sql, ctx)
+
+    # Convert plan to JSON
+    json_str = plan.to_json()
+    assert isinstance(json_str, str)
+    assert "relations" in json_str  # basic sanity check

Review Comment:
   Hi @timsaucer,
   
   I was able to generate a JSON-formatted Substrait plan using my DataFusion implementation. Comparing it with Isthmus:
   
   - Both include relations, project expressions, virtualTable values, and correct column mappings.
   - Minor differences exist: DataFusion uses i64 instead of i32, column names differ, and optional metadata (extensions, typeAliases) are not included — these do not affect compatibility. 
   - DuckDB currently does not provide a working JSON inspection method due to extension availability issues, so direct JSON comparison via DuckDB isn’t possible.
   
   Overall, the DataFusion output is compatible and can be used for reproducibility and validation.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#issuecomment-3897068057

   Thank you for the PR. There was an issue where the internal function name differed from the wrapper, so unit tests were failing. I pushed an update correcting this and making a change to the unit test to focus on the json representation of the logical plan rather than serialization and deserialization.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "Prathamesh9284 (via GitHub)" <gi...@apache.org>.
Prathamesh9284 commented on code in PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#discussion_r2794954967


##########
python/datafusion/substrait.py:
##########
@@ -137,6 +155,29 @@ def deserialize_bytes(proto_bytes: bytes) -> Plan:
         """
         return Plan(substrait_internal.Serde.deserialize_bytes(proto_bytes))
 
+    @staticmethod
+    def serialize_json(sql: str, ctx: SessionContext, path: str | pathlib.Path) -> None:
+        """Serialize a SQL query to a JSON-formatted Substrait plan file.
+
+        Args:
+            sql: SQL query to serialize.
+            ctx: SessionContext to use.
+            path: Path to write the JSON-formatted Substrait plan to.
+        """
+        return substrait_internal.Serde.serialize_json(sql, ctx.ctx, str(path))
+
+    @staticmethod
+    def deserialize_json(path: str | pathlib.Path) -> Plan:
+        """Deserialize a Substrait plan from a JSON file.

Review Comment:
   Following up on my earlier comment if we keep JSON handling on Plan, would it make sense for from_json() to accept either a JSON string or a file path? That might simplify things instead of having separate helpers on Serde. Curious what you think.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "Prathamesh9284 (via GitHub)" <gi...@apache.org>.
Prathamesh9284 commented on code in PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#discussion_r2794966854


##########
python/datafusion/substrait.py:
##########
@@ -137,6 +155,29 @@ def deserialize_bytes(proto_bytes: bytes) -> Plan:
         """
         return Plan(substrait_internal.Serde.deserialize_bytes(proto_bytes))
 
+    @staticmethod
+    def serialize_json(sql: str, ctx: SessionContext, path: str | pathlib.Path) -> None:
+        """Serialize a SQL query to a JSON-formatted Substrait plan file.

Review Comment:
   Yes, in the current implementation it writes a JSON-formatted Substrait plan to a file. I used “serialize” in the same sense as the existing binary serialize, but I agree the naming may not be very clear here.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "Prathamesh9284 (via GitHub)" <gi...@apache.org>.
Prathamesh9284 commented on PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#issuecomment-3897849376

   Thank you for fixing the internal function name mismatch and adjusting the unit test. I did notice that one of the CI builds failed. The failure appears to be related to a Clippy lint (redundant_closure). I’ve pushed a small fix replacing the redundant closure with the function directly (map_err(to_datafusion_err)), which should resolve the lint issue across environments.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "Prathamesh9284 (via GitHub)" <gi...@apache.org>.
Prathamesh9284 commented on code in PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#discussion_r2794985290


##########
python/datafusion/substrait.py:
##########
@@ -67,6 +67,24 @@ def encode(self) -> bytes:
         """
         return self.plan_internal.encode()
 
+    def to_json(self) -> str:
+        """Serialize the plan to a JSON string.
+
+        Returns a JSON string following the Protobuf JSON Mapping.

Review Comment:
   I agree the wording isn’t very clear. I’ll rephrase it to make it clearer that this just returns the Plan in its standard JSON form.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#issuecomment-3900135619

   Thank you for the work on this!
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "Prathamesh9284 (via GitHub)" <gi...@apache.org>.
Prathamesh9284 commented on PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#issuecomment-3892511969

   Hi @timsaucer 
   
   I have added pytest to validate cross-library compatibility. On substrait.py ines 94–99 parse the JSON using the official Substrait protobuf library (plan_pb2) via json_format.Parse, which would fail if the JSON didn’t conform to the Substrait protobuf schema. It then re-serializes to binary and asserts byte-for-byte equality with the original DataFusion output. This ensures any tool that supports protobuf-JSON can consume it correctly.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "Prathamesh9284 (via GitHub)" <gi...@apache.org>.
Prathamesh9284 commented on code in PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#discussion_r2794934677


##########
python/datafusion/substrait.py:
##########
@@ -137,6 +155,29 @@ def deserialize_bytes(proto_bytes: bytes) -> Plan:
         """
         return Plan(substrait_internal.Serde.deserialize_bytes(proto_bytes))
 
+    @staticmethod
+    def serialize_json(sql: str, ctx: SessionContext, path: str | pathlib.Path) -> None:

Review Comment:
   I was originally trying to keep it consistent with Serde.serialize, which writes the binary plan to a file. But I’m also fine with just returning a JSON string from Plan.to_json() and letting users handle writing to a file themselves. Do you have a preference for which approach fits better with the project?



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] feat: add support for generating JSON formatted substrait plan [datafusion-python]

Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on code in PR #1376:
URL: https://github.com/apache/datafusion-python/pull/1376#discussion_r2794347792


##########
python/datafusion/substrait.py:
##########
@@ -137,6 +155,29 @@ def deserialize_bytes(proto_bytes: bytes) -> Plan:
         """
         return Plan(substrait_internal.Serde.deserialize_bytes(proto_bytes))
 
+    @staticmethod
+    def serialize_json(sql: str, ctx: SessionContext, path: str | pathlib.Path) -> None:

Review Comment:
   Is the preferred method here to write to a file or to just produce a string output? Maybe the author of the issue has an opinion on best use case.



##########
src/errors.rs:
##########
@@ -34,6 +36,8 @@ pub enum PyDataFusionError {
     Common(String),
     PythonError(PyErr),
     EncodeError(EncodeError),
+    DecodeError(DecodeError),
+    SerdeJsonError(SerdeJsonError),

Review Comment:
   Recommend removing SerdeJsonError and mapping to EncodeError or DecodeError. Alternatively putting into DataFusion execution error.



##########
python/datafusion/substrait.py:
##########
@@ -137,6 +155,29 @@ def deserialize_bytes(proto_bytes: bytes) -> Plan:
         """
         return Plan(substrait_internal.Serde.deserialize_bytes(proto_bytes))
 
+    @staticmethod
+    def serialize_json(sql: str, ctx: SessionContext, path: str | pathlib.Path) -> None:
+        """Serialize a SQL query to a JSON-formatted Substrait plan file.

Review Comment:
   Is the output a json file? I'm trying to understand the "serialize" in this context.



##########
python/tests/test_substrait.py:
##########
@@ -74,3 +74,48 @@ def test_substrait_file_serialization(ctx, tmp_path, path_to_str):
     expected_actual_plan = ss.Consumer.from_substrait_plan(ctx, actual_plan)
 
     assert str(expected_logical_plan) == str(expected_actual_plan)
+
+def test_substrait_plan_to_from_json(ctx):
+    # Create a simple plan
+    sql = "SELECT * FROM t"
+    batch = pa.RecordBatch.from_arrays(
+        [pa.array([1, 2, 3]), pa.array([4, 5, 6])],
+        names=["a", "b"],
+    )
+    ctx.register_record_batches("t", [[batch]])
+
+    # Serialize to plan
+    plan = ss.Serde.serialize_to_plan(sql, ctx)
+
+    # Convert plan to JSON
+    json_str = plan.to_json()
+    assert isinstance(json_str, str)
+    assert "relations" in json_str  # basic sanity check

Review Comment:
   If these conversions to json are stable as the issue suggests, then I would think we could compare the entire output, right? What do the other libraries produce?



##########
python/datafusion/substrait.py:
##########
@@ -67,6 +67,24 @@ def encode(self) -> bytes:
         """
         return self.plan_internal.encode()
 
+    def to_json(self) -> str:
+        """Serialize the plan to a JSON string.
+
+        Returns a JSON string following the Protobuf JSON Mapping.

Review Comment:
   I find the statement "Returns a JSON string following the Protobuf JSON Mapping" to be hard to understand. What exactly does this mean? Is this LLM generated?



##########
python/datafusion/substrait.py:
##########
@@ -137,6 +155,29 @@ def deserialize_bytes(proto_bytes: bytes) -> Plan:
         """
         return Plan(substrait_internal.Serde.deserialize_bytes(proto_bytes))
 
+    @staticmethod
+    def serialize_json(sql: str, ctx: SessionContext, path: str | pathlib.Path) -> None:
+        """Serialize a SQL query to a JSON-formatted Substrait plan file.
+
+        Args:
+            sql: SQL query to serialize.
+            ctx: SessionContext to use.
+            path: Path to write the JSON-formatted Substrait plan to.
+        """
+        return substrait_internal.Serde.serialize_json(sql, ctx.ctx, str(path))
+
+    @staticmethod
+    def deserialize_json(path: str | pathlib.Path) -> Plan:
+        """Deserialize a Substrait plan from a JSON file.

Review Comment:
   Similarly, not sure if this needs to be a path to a file that we read or expect to pass in a string. Since it's python it's fairly easy to handle either/both



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org