msyavuz commented on code in PR #36277:
URL: https://github.com/apache/superset/pull/36277#discussion_r2565259365
##########
superset-frontend/src/SqlLab/actions/sqlLab.js:
##########
@@ -914,11 +914,26 @@ export function
queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) {
export function formatQuery(queryEditor) {
return function (dispatch, getState) {
- const { sql } = getUpToDateQuery(getState(), queryEditor);
+ const qe = getUpToDateQuery(getState(), queryEditor);
+ const { sql, dbId, templateParams } = qe;
+ const body = { sql };
+
+ // Include database_id and template_params if available for Jinja
processing
+ if (dbId) {
+ body.database_id = dbId;
+ }
+ if (templateParams) {
+ // Send templateParams as a JSON string to match the backend schema
+ body.template_params =
+ typeof templateParams === 'string'
+ ? templateParams
+ : JSON.stringify(templateParams);
+ }
+
return SupersetClient.post({
endpoint: `/api/v1/sqllab/format_sql/`,
// TODO (betodealmeida): pass engine as a parameter for better formatting
Review Comment:
Is this TODO relevant here?
##########
superset/sqllab/api.py:
##########
@@ -234,7 +234,34 @@ def format_sql(self) -> FlaskResponse:
"""
try:
model = self.format_model_schema.load(request.json)
- result = SQLScript(model["sql"], model.get("engine")).format()
+ sql = model["sql"]
+ template_params = model.get("template_params")
+ database_id = model.get("database_id")
+
+ # Process Jinja templates if template_params and database_id are
provided
+ if template_params and database_id is not None:
+ database = DatabaseDAO.find_by_id(database_id)
+ if database:
+ try:
+ template_params = (
+ json.loads(template_params)
+ if isinstance(template_params, str)
+ else template_params
+ )
+ except json.JSONDecodeError:
+ logger.warning(
+ "Invalid template parameter %s. Skipping
processing",
+ str(template_params),
+ )
+ template_params = {}
+
+ if template_params:
+ template_processor =
get_template_processor(database=database)
+ sql = template_processor.process_template(
+ sql, **template_params
+ )
Review Comment:
Should this be inside the `try` block?
##########
superset-frontend/src/SqlLab/actions/sqlLab.js:
##########
@@ -914,11 +914,26 @@ export function
queryEditorSetAndSaveSql(targetQueryEditor, sql, queryId) {
export function formatQuery(queryEditor) {
return function (dispatch, getState) {
- const { sql } = getUpToDateQuery(getState(), queryEditor);
+ const qe = getUpToDateQuery(getState(), queryEditor);
+ const { sql, dbId, templateParams } = qe;
+ const body = { sql };
Review Comment:
Can we still do this inline? are we using the variable `qe` anywhere else?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
