You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "luca-p-castelli (via GitHub)" <gi...@apache.org> on 2026/04/27 21:21:31 UTC

[PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

luca-p-castelli opened a new pull request, #28054:
URL: https://github.com/apache/flink/pull/28054

   ## What is the purpose of the change
   
   Today `COMPILE PLAN` requires a file path and writes the compiled `ExecNodeGraph` JSON to disk:
   
   ```sql
   COMPILE PLAN '/path/to/plan.json' FOR INSERT INTO sink SELECT * FROM src;
   ```
   
   This requires the caller to share a filesystem with the executor. For tooling built on the SQL Gateway (CLIs, IDEs, CI systems) this adds filesystem-coordination complexity, and for remote SQL Gateway deployments it does not work at all — the file lands on the gateway host, not the caller's host.
   
   This PR makes the file path optional. When omitted, the compiled plan is returned inline as a single-row, single-column `STRING` result set with `ResultKind.SUCCESS_WITH_CONTENT` — the same result-set pattern already used by `EXPLAIN`:
   
   ```sql
   COMPILE PLAN FOR INSERT INTO sink SELECT * FROM src;
   -- returns plan JSON via fetchResults
   ```
   
   Fully backward compatible: the existing `COMPILE PLAN '<path>' [IF NOT EXISTS] FOR <dml>` syntax and behavior are unchanged. This is an internal-only change at the API level — both `SqlCompilePlan` and `CompilePlanOperation` are annotated `@Internal`; no new public API surface, no new REST endpoints.
   
   Discussion thread: https://lists.apache.org/thread/j2z9obdyr9k0078jj1cd6tjj5nvbdk86
   
   ## Brief change log
   
   - **Parser** (`parserImpls.ftl`): wrap the file-path branch in `LOOKAHEAD(2)` so the grammar accepts both `COMPILE PLAN '<path>' [IF NOT EXISTS] FOR <dml>` and `COMPILE PLAN FOR <dml>`.
   - **AST** (`SqlCompilePlan`): `planFile` is now `@Nullable`; `unparse` emits the path and `IF NOT EXISTS` only when present.
   - **Operation** (`CompilePlanOperation`): `filePath` is now `@Nullable`; constructor adds a `Preconditions.checkArgument` rejecting `IF NOT EXISTS` when no file path is specified.
   - **API** (`TableEnvironmentImpl`): when the file path is null, return the plan via `TableResultUtils.buildStringArrayResult("result", new String[]{compiledPlan.asJsonString()})` — mirroring how `EXPLAIN` returns its single `STRING` column. Extracted a shared `compileOperationToPlan` helper used by both the file-based and inline paths.
   - **Tests**: parser round-trip tests in `FlinkSqlParserImplTest` for inline `INSERT` and inline `STATEMENT SET`; integration tests in `CompiledPlanITCase` (`testCompilePlanInline`, `testCompilePlanInlineWithStatementSet`) verifying `ResultKind.SUCCESS_WITH_CONTENT`, the presence of `flinkVersion`, and the expected sink identifiers in the returned JSON.
   
   ## Verifying this change
   
   This change added tests and can be verified as follows:
   
   - `FlinkSqlParserImplTest#compilePlan` — extended with parse/unparse round-trip cases for both inline forms (single `INSERT`, `STATEMENT SET`).
   - `CompiledPlanITCase#testCompilePlanInline` — verifies single-`INSERT` inline form: result kind is `SUCCESS_WITH_CONTENT`, returned JSON contains `flinkVersion`, source-scan, and sink exec nodes.
   - `CompiledPlanITCase#testCompilePlanInlineWithStatementSet` — verifies `STATEMENT SET` inline form: returned JSON contains both sink identifiers (`sinkA`, `sinkB`).
   - All existing file-based `COMPILE PLAN` tests pass unchanged.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): **no**
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: **no** (both `SqlCompilePlan` and `CompilePlanOperation` are `@Internal`)
     - The serializers: **no**
     - The runtime per-record code paths (performance sensitive): **no**
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: **no**
     - The S3 file system connector: **no**
   
   ## Documentation
   
     - Does this pull request introduce a new feature? **yes** (extends existing `COMPILE PLAN` syntax with an optional file path)
     - If yes, how is the feature documented? **JavaDocs** (`SqlCompilePlan` and `CompilePlanOperation` javadocs updated to document the optional-path semantics). Happy to extend the SQL reference documentation in a follow-up commit on this PR if reviewers prefer that to land here.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Code (Claude Opus 4.7)
   
   Generated-by: Claude Code (Claude Opus 4.7)


-- 
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: issues-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

Posted by "ryanvanhuuksloot (via GitHub)" <gi...@apache.org>.
ryanvanhuuksloot commented on code in PR #28054:
URL: https://github.com/apache/flink/pull/28054#discussion_r3453027705


##########
docs/content/docs/concepts/sql-table-concepts/overview.md:
##########
@@ -278,20 +278,24 @@ Flink SQL> COMPILE PLAN 'file:///path/to/plan.json' FOR INSERT INTO enriched_ord
 - SQL Syntax
 
     ```sql
-    COMPILE PLAN [IF NOT EXISTS] <plan_file_path> FOR <insert_statement>|<statement_set>;
-    
+    -- Write the compiled plan to a file:
+    COMPILE PLAN '<plan_file_path>' [IF NOT EXISTS] FOR <insert_statement>|<statement_set>;
+
+    -- Or return the compiled plan inline as a single-row, single-column STRING result set:
+    COMPILE PLAN FOR <insert_statement>|<statement_set>;

Review Comment:
   @davidradl I'd say that the write syntax is the strange syntax here. It is the same reason that EXPLAIN doesn't write to a file. Someone must have added it as a shortcut but SQL shouldn't write to a filepath like that.
   
   If anything, I'd like to remove that syntax. I'm of the opinion that this COMPILE PLAN that returns the plan is the correct implementation and we should merge this.
   
   What do 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: issues-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

Posted by "luca-p-castelli (via GitHub)" <gi...@apache.org>.
luca-p-castelli commented on code in PR #28054:
URL: https://github.com/apache/flink/pull/28054#discussion_r3150608502


##########
docs/content/docs/concepts/sql-table-concepts/overview.md:
##########
@@ -278,20 +278,24 @@ Flink SQL> COMPILE PLAN 'file:///path/to/plan.json' FOR INSERT INTO enriched_ord
 - SQL Syntax
 
     ```sql
-    COMPILE PLAN [IF NOT EXISTS] <plan_file_path> FOR <insert_statement>|<statement_set>;

Review Comment:
   Typo in the current docs. `[IF NOT EXISTS]` should be after the file path.



-- 
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: issues-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

Posted by "ryanvanhuuksloot (via GitHub)" <gi...@apache.org>.
ryanvanhuuksloot commented on code in PR #28054:
URL: https://github.com/apache/flink/pull/28054#discussion_r3453027705


##########
docs/content/docs/concepts/sql-table-concepts/overview.md:
##########
@@ -278,20 +278,24 @@ Flink SQL> COMPILE PLAN 'file:///path/to/plan.json' FOR INSERT INTO enriched_ord
 - SQL Syntax
 
     ```sql
-    COMPILE PLAN [IF NOT EXISTS] <plan_file_path> FOR <insert_statement>|<statement_set>;
-    
+    -- Write the compiled plan to a file:
+    COMPILE PLAN '<plan_file_path>' [IF NOT EXISTS] FOR <insert_statement>|<statement_set>;
+
+    -- Or return the compiled plan inline as a single-row, single-column STRING result set:
+    COMPILE PLAN FOR <insert_statement>|<statement_set>;

Review Comment:
   @davidradl I'd argue that the write syntax is the strange syntax here. It is the same reason that EXPLAIN doesn't write to a file. Someone must have added it as a shortcut but SQL shouldn't write to a filepath like that.
   
   If anything, I'd like to remove that syntax. I'm of the opinion that this COMPILE PLAN that returns the plan is the correct implementation and we should merge this.
   
   What do 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: issues-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #28054:
URL: https://github.com/apache/flink/pull/28054#discussion_r3287831354


##########
docs/content/docs/concepts/sql-table-concepts/overview.md:
##########
@@ -278,20 +278,24 @@ Flink SQL> COMPILE PLAN 'file:///path/to/plan.json' FOR INSERT INTO enriched_ord
 - SQL Syntax
 
     ```sql
-    COMPILE PLAN [IF NOT EXISTS] <plan_file_path> FOR <insert_statement>|<statement_set>;
-    
+    -- Write the compiled plan to a file:
+    COMPILE PLAN '<plan_file_path>' [IF NOT EXISTS] FOR <insert_statement>|<statement_set>;
+
+    -- Or return the compiled plan inline as a single-row, single-column STRING result set:
+    COMPILE PLAN FOR <insert_statement>|<statement_set>;

Review Comment:
   It looks strange to have COMPILE PLAN to mean write or read depending on the subsequent syntax. I am not sure about the SQL precedent for 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: issues-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

Posted by "luca-p-castelli (via GitHub)" <gi...@apache.org>.
luca-p-castelli commented on code in PR #28054:
URL: https://github.com/apache/flink/pull/28054#discussion_r3290524127


##########
docs/content/docs/concepts/sql-table-concepts/overview.md:
##########
@@ -278,20 +278,24 @@ Flink SQL> COMPILE PLAN 'file:///path/to/plan.json' FOR INSERT INTO enriched_ord
 - SQL Syntax
 
     ```sql
-    COMPILE PLAN [IF NOT EXISTS] <plan_file_path> FOR <insert_statement>|<statement_set>;
-    
+    -- Write the compiled plan to a file:
+    COMPILE PLAN '<plan_file_path>' [IF NOT EXISTS] FOR <insert_statement>|<statement_set>;
+
+    -- Or return the compiled plan inline as a single-row, single-column STRING result set:
+    COMPILE PLAN FOR <insert_statement>|<statement_set>;

Review Comment:
   Thanks for the feedback, David. 
   
   I agree this is worth clarifying, but I don’t think of the distinction as write vs read. COMPILE PLAN always compiles the supplied INSERT / STATEMENT SET; the optional path only selects the output destination.
   
   With a path, the existing behaviour is unchanged: the compiled plan JSON is written to a Flink-supported filesystem. Without a path, the same compiled plan JSON is returned to the client as a result set. EXECUTE PLAN remains the statement that reads a compiled plan.
   
   This also mirrors the Table API, where compilePlanSql(...) returns a CompiledPlan, and callers can choose asJsonString() or writeToFile(...).



-- 
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: issues-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

Posted by "spuru9 (via GitHub)" <gi...@apache.org>.
spuru9 commented on PR #28054:
URL: https://github.com/apache/flink/pull/28054#issuecomment-4358173598

   cc @kumar-mallikarjuna 


-- 
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: issues-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #28054:
URL: https://github.com/apache/flink/pull/28054#issuecomment-4330569939

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "bca15b1865aab94719e6f2016ed05c8ca5bc22b3",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "bca15b1865aab94719e6f2016ed05c8ca5bc22b3",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * bca15b1865aab94719e6f2016ed05c8ca5bc22b3 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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: issues-unsubscribe@flink.apache.org

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


Re: [PR] [FLINK-39556][table] Make file path optional in COMPILE PLAN to return plan inline as a result set [flink]

Posted by "luca-p-castelli (via GitHub)" <gi...@apache.org>.
luca-p-castelli commented on PR #28054:
URL: https://github.com/apache/flink/pull/28054#issuecomment-4330552056

   @flinkbot run azure


-- 
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: issues-unsubscribe@flink.apache.org

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