You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/05/18 15:21:03 UTC

[PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

rusackas opened a new pull request, #40224:
URL: https://github.com/apache/superset/pull/40224

   ### SUMMARY
   
   This is a **test-only PR** opened as a TDD-style validation of issue #25839.
   
   #25839 (filed 2023-11) reports that clicking *SYNC COLUMNS FROM SOURCE* on a virtual dataset containing Jinja (e.g. \`{{ from_dttm }}\`) produces an *Invalid SQL* error because Jinja is not rendered before the SQL parser sees it.
   
   This PR adds one regression test on \`get_virtual_table_metadata\`:
   
   1. **\`test_get_virtual_table_metadata_renders_jinja\`** — constructs a virtual dataset with \`'{{ from_dttm }}'\` in its SQL, mocks the template processor to return rendered SQL, calls \`get_virtual_table_metadata\`, and asserts that (a) no exception is raised, (b) the template processor was called with the raw Jinja SQL.
   
   ### How to interpret CI
   
   - **CI green** → \`get_virtual_table_metadata\` correctly runs the template processor before SQL parsing; merging closes #25839 and locks in the regression guard.
   - **CI red** → Jinja-rendering step is missing or out of order. Likely fix region: \`superset/connectors/sqla/utils.py::get_virtual_table_metadata\` lines 105–110.
   
   ### TESTING INSTRUCTIONS
   
   \`\`\`bash
   pytest tests/unit_tests/connectors/sqla/utils_test.py::test_get_virtual_table_metadata_renders_jinja -v
   \`\`\`
   
   ### ADDITIONAL INFORMATION
   
   - [x] Has associated issue: closes #25839
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.

codecov[bot] commented on PR #40224:
URL: https://github.com/apache/superset/pull/40224#issuecomment-4479648194

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40224?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 63.39%. Comparing base ([`70419e9`](https://app.codecov.io/gh/apache/superset/commit/70419e9d8f81477026eba57fb1fc32e7abbc0e65?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`6e84a27`](https://app.codecov.io/gh/apache/superset/commit/6e84a2762d6ca14a038044a042a7746b02b17c7f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 3 commits behind head on master.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #40224      +/-   ##
   ==========================================
   - Coverage   64.15%   63.39%   -0.76%     
   ==========================================
     Files        2591     2591              
     Lines      138214   138214              
     Branches    32056    32056              
   ==========================================
   - Hits        88670    87621    -1049     
   - Misses      48014    49057    +1043     
   - Partials     1530     1536       +6     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/40224/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/40224/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.45% <ø> (ø)` | |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/40224/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/40224/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/40224/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.14% <ø> (ø)` | |
   | [python](https://app.codecov.io/gh/apache/superset/pull/40224/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.99% <ø> (-1.67%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/40224/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.86% <ø> (ø)` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/40224/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/40224?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #40224:
URL: https://github.com/apache/superset/pull/40224#issuecomment-4479173596

   <!-- Bito Reply -->
   The PR comment suggests that the regression test for Jinja rendering in virtual datasets is incomplete. The test currently verifies that the `get_columns_description` function returns the expected column list but does not check whether it was called with the rendered SQL. To fix this, you should capture the mock and assert that it was called with `rendered_sql` as the query argument. Here's how you can implement the fix concisely in `tests/unit_tests/connectors/sqla/utils_test.py`:
   
   **tests/unit_tests/connectors/sqla/utils_test.py**
   ```
   mocker.patch(
       "superset.connectors.sqla.utils.get_columns_description",
       return_value=[{"name": "rendered_col", "type": "INTEGER"}],
       new_callable=mocker.MagicMock
   )
   
   # ... existing test code ...
   
   # Add this assertion to verify the rendered SQL is used
   get_columns_description_mock = mocker.patch(
       "superset.connectors.sqla.utils.get_columns_description",
       return_value=[{"name": "rendered_col", "type": "INTEGER"}],
       new_callable=mocker.MagicMock
   )
   
   assert get_columns_description_mock.call_args[1]["query"] == rendered_sql
   ```


-- 
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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #40224:
URL: https://github.com/apache/superset/pull/40224#issuecomment-4479112373

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.

codeant-ai-for-open-source[bot] commented on code in PR #40224:
URL: https://github.com/apache/superset/pull/40224#discussion_r3260080523


##########
tests/unit_tests/connectors/sqla/utils_test.py:
##########
@@ -137,3 +137,38 @@ def test_get_virtual_table_metadata_multiple(mocker: MockerFixture) -> None:
     with pytest.raises(SupersetSecurityException) as excinfo:
         get_virtual_table_metadata(dataset)
     assert str(excinfo.value) == "Only single queries supported"
+
+
+def test_get_virtual_table_metadata_renders_jinja(mocker: MockerFixture) -> None:
+    """Regression for #25839: Jinja templates in a virtual dataset's SQL must
+    be rendered via the template processor before SQL parsing. Otherwise the
+    raw Jinja tokens reach sqlglot and the parser rejects them as a syntax
+    error (the user-visible symptom is "Invalid SQL" when clicking
+    "SYNC COLUMNS FROM SOURCE" on a dataset that uses {{ from_dttm }} etc.).
+    """
+    mocker.patch(
+        "superset.connectors.sqla.utils.get_columns_description",
+        return_value=[{"name": "rendered_col", "type": "INTEGER"}],
+    )
+
+    raw_sql = "SELECT * FROM tbl WHERE ts > '{{ from_dttm }}'"
+    rendered_sql = "SELECT * FROM tbl WHERE ts > '2024-01-01 00:00:00'"
+
+    dataset = mocker.MagicMock(sql=raw_sql)
+    dataset.database.db_engine_spec.engine = "postgresql"
+    dataset.template_params_dict = {}
+    dataset.get_template_processor().process_template.return_value = rendered_sql
+
+    # If Jinja rendering is skipped, sqlglot tries to parse the raw {{ ... }}
+    # and raises SupersetGenericDBErrorException / SupersetParseError.
+    assert get_virtual_table_metadata(dataset) == [
+        {"name": "rendered_col", "type": "INTEGER"}
+    ]

Review Comment:
   **Suggestion:** The regression test never verifies which SQL string is passed into `get_columns_description`, so it can still pass if the function renders Jinja for parsing but then sends the original unrendered SQL downstream. Capture the patched `get_columns_description` mock and assert it was called with `rendered_sql` as the query argument to ensure the rendered statement is used end-to-end. [incomplete implementation]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Virtual dataset metadata may still use unrendered Jinja SQL.
   - ⚠️ Column sync regression guard misses downstream SQL argument.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Open `tests/unit_tests/connectors/sqla/utils_test.py` and inspect
   `test_get_virtual_table_metadata_renders_jinja` at lines 149–166, where
   `mocker.patch("superset.connectors.sqla.utils.get_columns_description",
   return_value=[{"name": "rendered_col", "type": "INTEGER"}])` stubs
   `get_columns_description` to return a fixed list regardless of the `query` argument.
   
   2. Open `superset/connectors/sqla/utils.py` and inspect `get_virtual_table_metadata` at
   lines 99–137: it currently calls `sql =
   dataset.get_template_processor().process_template(dataset.sql,
   **dataset.template_params_dict)` (around line 108) and then passes this `sql` into both
   `SQLScript(sql, engine=db_engine_spec.engine)` and
   `get_columns_description(dataset.database, dataset.catalog, dataset.schema, sql)` (around
   lines 137–62).
   
   3. Consider a regression consistent with issue #25839 where only the parsing path uses
   rendered SQL: change `get_virtual_table_metadata` so that `SQLScript` continues to receive
   the rendered `sql`, but the final `get_columns_description(...)` call (lines 137–62) is
   modified to pass `dataset.sql` instead of `sql`, meaning that metadata retrieval runs
   against the raw `"SELECT * FROM tbl WHERE ts > '{{ from_dttm }}'"`.
   
   4. In this regressed state, a real virtual dataset using Jinja (e.g. via
   `SqlaTable.external_metadata` at `superset/connectors/sqla/models.py:8–16`) would still
   fail when the user clicks "SYNC COLUMNS FROM SOURCE", because the DB backend sees
   unrendered Jinja; however, running `pytest
   tests/unit_tests/connectors/sqla/utils_test.py::test_get_virtual_table_metadata_renders_jinja
   -v` will still pass since the test only checks the returned column list and never asserts
   that `get_columns_description` was called with `rendered_sql` as its `query` argument.
   ```
   </details>
   
   [Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20tests%2Funit_tests%2Fconnectors%2Fsqla%2Futils_test.py%0A%2A%2ALine%3A%2A%2A%20149%3A166%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncomplete%20Implementation%3A%20The%20regression%20test%20never%20verifies%20which%20SQL%20string%20is%20passed%20into%20%60get_columns_description%60%2C%20so%20it%20can%20still%20pass%20if%20the%20function%20renders%20Jinja%20for%20parsing%20but%20then%20sends%20the%20original%20unrendered%20SQL%20downstream.%20Capture%20the%20patched%20%60get_columns_description%60%20mock%20and%20assert%20it%20was%20called%20with%20%60rendered_sql%60%20as%20the%20query%20argument%20to%20ensure%20the%20rendered%20statement%20is%20used%20end-to-end.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%2
 0please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20tests%2Funit_tests%2Fconnectors%2Fsqla%2Futils_test.py%0A%2A%2ALine%3A%2A%2A%20149%3A166%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncomplete%20Implementation%3A%20The%20regression%20test%20never%20verifies%20which%20SQL%20string%20is%20passed%20into%20%60get_columns_description%60%2C%20so%20it%20can%20still%20pass%20if%20the%20function%20renders%20Jinja%20for%20parsing%20but%20then%20sends%20the%20original%20unrendered%20SQL%20downstream.%20Capture%20the%20patched%2
 0%60get_columns_description%60%20mock%20and%20assert%20it%20was%20called%20with%20%60rendered_sql%60%20as%20the%20query%20argument%20to%20ensure%20the%20rendered%20statement%20is%20used%20end-to-end.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** tests/unit_tests/connectors/sqla/utils_test.py
   **Line:** 149:166
   **Comment:**
   	*Incomplete Implementation: The regression test never verifies which SQL string is passed into `get_columns_description`, so it can still pass if the function renders Jinja for parsing but then sends the original unrendered SQL downstream. Capture the patched `get_columns_description` mock and assert it was called with `rendered_sql` as the query argument to ensure the rendered statement is used end-to-end.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40224&comment_hash=6a2cf77c803bd337a4a28a14ebee12941757d48282fa4ecdc5edd66a1c672775&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40224&comment_hash=6a2cf77c803bd337a4a28a14ebee12941757d48282fa4ecdc5edd66a1c672775&reaction=dislike'>👎</a>



-- 
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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.

bito-code-review[bot] commented on PR #40224:
URL: https://github.com/apache/superset/pull/40224#issuecomment-4495494402

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas merged PR #40224:
URL: https://github.com/apache/superset/pull/40224


-- 
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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.

rusackas commented on code in PR #40224:
URL: https://github.com/apache/superset/pull/40224#discussion_r3262199580


##########
tests/unit_tests/connectors/sqla/utils_test.py:
##########
@@ -137,3 +137,38 @@ def test_get_virtual_table_metadata_multiple(mocker: MockerFixture) -> None:
     with pytest.raises(SupersetSecurityException) as excinfo:
         get_virtual_table_metadata(dataset)
     assert str(excinfo.value) == "Only single queries supported"
+
+
+def test_get_virtual_table_metadata_renders_jinja(mocker: MockerFixture) -> None:
+    """Regression for #25839: Jinja templates in a virtual dataset's SQL must
+    be rendered via the template processor before SQL parsing. Otherwise the
+    raw Jinja tokens reach sqlglot and the parser rejects them as a syntax
+    error (the user-visible symptom is "Invalid SQL" when clicking
+    "SYNC COLUMNS FROM SOURCE" on a dataset that uses {{ from_dttm }} etc.).
+    """
+    mocker.patch(
+        "superset.connectors.sqla.utils.get_columns_description",
+        return_value=[{"name": "rendered_col", "type": "INTEGER"}],
+    )
+
+    raw_sql = "SELECT * FROM tbl WHERE ts > '{{ from_dttm }}'"
+    rendered_sql = "SELECT * FROM tbl WHERE ts > '2024-01-01 00:00:00'"
+
+    dataset = mocker.MagicMock(sql=raw_sql)
+    dataset.database.db_engine_spec.engine = "postgresql"
+    dataset.template_params_dict = {}
+    dataset.get_template_processor().process_template.return_value = rendered_sql
+
+    # If Jinja rendering is skipped, sqlglot tries to parse the raw {{ ... }}
+    # and raises SupersetGenericDBErrorException / SupersetParseError.
+    assert get_virtual_table_metadata(dataset) == [
+        {"name": "rendered_col", "type": "INTEGER"}
+    ]

Review Comment:
   Good catch — fixed in 8bb3082f. Captured the patched `get_columns_description` mock and now assert that its `query` argument equals `rendered_sql`. A regression where rendering is used for sqlglot parsing but the raw SQL leaks downstream to the metadata call would now fail this assertion.



-- 
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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] test(datasets): regression test for Jinja not rendered on sync columns (#25839) [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.

netlify[bot] commented on PR #40224:
URL: https://github.com/apache/superset/pull/40224#issuecomment-4482290299

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 8bb3082ffdc804efc25f093bc53257e1b0d8a436 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a0b81527803600008b7bbf7 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40224--superset-docs-preview.netlify.app](https://deploy-preview-40224--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQwMjI0LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.hvdhW5WvTs2mQgUJ_qkfFb_LrmqVAleYN6sWJAGywkA)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcrJDYAgEEDRXuYsESUs2oFlMIARwhbRgzH2Lv_0D--FepZUL1gBBsBTZ3P0v6wdfWu3IzNXbCHB56BJe7IhpsQ75da1dTWWZ7PdC01RTXyWijJBewol4i7h-wE) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-40224--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org