diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.ts b/superset-frontend/src/SqlLab/actions/sqlLab.test.ts index ace26784469..75a67a296ab 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.ts +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.ts @@ -103,7 +103,7 @@ describe('async actions', () => { beforeEach(() => { dispatch = jest.fn(); fetchMock.removeRoute(fetchQueryEndpoint); - fetchMock.get( + fetchMock.post( fetchQueryEndpoint, JSON.stringify({ data: mockBigNumber, @@ -450,6 +450,23 @@ describe('async actions', () => { }); }); + test('sends the results key in the POST body and not the URL', () => { + expect.assertions(4); + + return makeRequest().then(() => { + const calls = fetchMock.callHistory.calls(fetchQueryEndpoint); + expect(calls).toHaveLength(1); + const call = calls[0]; + expect(call.options.method?.toUpperCase()).toBe('POST'); + // The results key must not leak into the query string + expect(call.url).not.toContain(query.resultsKey); + expect(JSON.parse(call.options.body as string)).toEqual({ + key: query.resultsKey, + rows: null, + }); + }); + }); + /* oxlint-disable-next-line jest/no-disabled-tests */ test.skip('parses large number result without losing precision', () => makeRequest().then(() => { @@ -481,7 +498,7 @@ describe('async actions', () => { expect.assertions(1); fetchMock.removeRoute(fetchQueryEndpoint); - fetchMock.get( + fetchMock.post( fetchQueryEndpoint, { throws: { message: 'error text' } }, { name: fetchQueryEndpoint }, diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.ts b/superset-frontend/src/SqlLab/actions/sqlLab.ts index 7058471a55b..403f8cadaed 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.ts +++ b/superset-frontend/src/SqlLab/actions/sqlLab.ts @@ -482,14 +482,14 @@ export function fetchQueryResults( const { SQLLAB_QUERY_RESULT_TIMEOUT } = getState().common?.conf ?? {}; dispatch(requestQueryResults(query)); - const queryParams = rison.encode({ - key: query.resultsKey, - rows: displayLimit || null, - }); const timeout = timeoutInMs ?? SQLLAB_QUERY_RESULT_TIMEOUT; const controller = new AbortController(); - return SupersetClient.get({ - endpoint: `/api/v1/sqllab/results/?q=${queryParams}`, + return SupersetClient.post({ + endpoint: `/api/v1/sqllab/results/`, + jsonPayload: { + key: query.resultsKey, + rows: displayLimit || null, + }, parseMethod: 'json-bigint', ...(timeout && { timeout, signal: controller.signal }), }) diff --git a/superset/sqllab/api.py b/superset/sqllab/api.py index 3229773eaf7..5a729c2634a 100644 --- a/superset/sqllab/api.py +++ b/superset/sqllab/api.py @@ -56,6 +56,7 @@ from superset.sqllab.schemas import ( QueryExecutionResponseSchema, sql_lab_get_results_schema, SQLLabBootstrapSchema, + SqlLabResultsSchema, ) from superset.sqllab.sql_json_executer import ( ASynchronousSqlJsonExecutor, @@ -85,6 +86,7 @@ class SqlLabRestApi(BaseSupersetApi): estimate_model_schema = EstimateQueryCostSchema() execute_model_schema = ExecutePayloadSchema() format_model_schema = FormatQueryPayloadSchema() + results_model_schema = SqlLabResultsSchema() apispec_parameter_schemas = { "sql_lab_get_results_schema": sql_lab_get_results_schema, @@ -95,6 +97,7 @@ class SqlLabRestApi(BaseSupersetApi): ExecutePayloadSchema, FormatQueryPayloadSchema, QueryExecutionResponseSchema, + SqlLabResultsSchema, SQLLabBootstrapSchema, ) @@ -493,8 +496,62 @@ class SqlLabRestApi(BaseSupersetApi): $ref: '#/components/responses/500' """ params = kwargs["rison"] - key = params.get("key") - rows = params.get("rows") + return self._get_results_response( + key=params.get("key"), rows=params.get("rows") + ) + + @expose("/results/", methods=("POST",)) + @protect() + @permission_name("get_results") + @statsd_metrics + @requires_json + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get_results", + log_to_statsd=False, + ) + def post_results(self) -> FlaskResponse: + """Get the result of a SQL query execution. + --- + post: + summary: Get the result of a SQL query execution + description: >- + Accepts the cached results key (and optional row limit) in the + request body instead of the query string. + requestBody: + description: The cached results key and optional row limit + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/SqlLabResultsSchema' + responses: + 200: + description: SQL query execution result + content: + application/json: + schema: + $ref: '#/components/schemas/QueryExecutionResponseSchema' + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' + 404: + $ref: '#/components/responses/404' + 410: + $ref: '#/components/responses/410' + 500: + $ref: '#/components/responses/500' + """ + try: + params = self.results_model_schema.load(request.json) + except ValidationError as error: + return self.response_400(message=error.messages) + + return self._get_results_response(key=params["key"], rows=params.get("rows")) + + def _get_results_response(self, key: str, rows: Optional[int]) -> FlaskResponse: result = SqlExecutionResultsCommand(key=key, rows=rows).run() # Using pessimistic json serialization since some database drivers can return diff --git a/superset/sqllab/schemas.py b/superset/sqllab/schemas.py index 3c2413f8d3b..54db9f5514f 100644 --- a/superset/sqllab/schemas.py +++ b/superset/sqllab/schemas.py @@ -27,6 +27,18 @@ sql_lab_get_results_schema = { } +class SqlLabResultsSchema(Schema): + key = fields.String( + required=True, + metadata={"description": "The results key of the cached query results"}, + ) + rows = fields.Integer( + required=False, + allow_none=True, + metadata={"description": "The maximum number of rows to return"}, + ) + + class EstimateQueryCostSchema(Schema): database_id = fields.Integer( required=True, metadata={"description": "The database id"} diff --git a/tests/integration_tests/sql_lab/api_tests.py b/tests/integration_tests/sql_lab/api_tests.py index 4329946fe89..1ef2afbbd5b 100644 --- a/tests/integration_tests/sql_lab/api_tests.py +++ b/tests/integration_tests/sql_lab/api_tests.py @@ -474,6 +474,80 @@ class TestSqlLabApi(SupersetTestCase): app.config["RESULTS_BACKEND_USE_MSGPACK"] = use_msgpack + @mock.patch("superset.commands.sql_lab.results.results_backend_use_msgpack", False) + def test_post_results_matches_get(self): + """The POST endpoint returns the same payload as the GET endpoint and + accepts the results key (and optional row limit) via the request body.""" + from superset.commands.sql_lab import results as command + + command.results_backend = mock.Mock() + self.login(ADMIN_USERNAME) + + data = [{"col_0": i} for i in range(100)] + payload = { + "status": QueryStatus.SUCCESS, + "query": {"rows": 100}, + "data": data, + } + expected_full = {"status": "success", "query": {"rows": 100}, "data": data} + expected_limited = { + "status": "success", + "query": {"rows": 100}, + "data": data[:1], + "displayLimitReached": True, + } + + query_mock = mock.Mock() + query_mock.sql = "SELECT *" + query_mock.database = 1 + query_mock.schema = "superset" + + use_msgpack = app.config["RESULTS_BACKEND_USE_MSGPACK"] + app.config["RESULTS_BACKEND_USE_MSGPACK"] = False + serialized_payload = sql_lab._serialize_payload(payload, False) + compressed = utils.zlib_compress(serialized_payload) + command.results_backend.get.return_value = compressed + + with mock.patch("superset.commands.sql_lab.results.db") as mock_superset_db: + mock_superset_db.session.query().filter_by().one_or_none.return_value = ( + query_mock + ) + # GET (backward compatible) with the key in the query string + get_resp = json.loads( + self.get_resp( + f"/api/v1/sqllab/results/?q={rison.dumps({'key': 'key'})}" + ) + ) + # POST with the key in the request body + post_rv = self.client.post( + "/api/v1/sqllab/results/", + json={"key": "key"}, + ) + post_resp = json.loads(post_rv.data.decode("utf-8")) + # POST honors the optional row limit + post_limited_rv = self.client.post( + "/api/v1/sqllab/results/", + json={"key": "key", "rows": 1}, + ) + post_limited = json.loads(post_limited_rv.data.decode("utf-8")) + + assert post_rv.status_code == 200 + assert get_resp == expected_full + assert post_resp == expected_full + assert post_limited == expected_limited + + app.config["RESULTS_BACKEND_USE_MSGPACK"] = use_msgpack + + def test_post_results_requires_key(self): + """The POST endpoint rejects requests missing the required results key.""" + self.login(ADMIN_USERNAME) + + rv = self.client.post("/api/v1/sqllab/results/", json={}) + resp_data = json.loads(rv.data.decode("utf-8")) + + assert rv.status_code == 400 + assert resp_data == {"message": {"key": ["Missing data for required field."]}} + @mock.patch("superset.models.sql_lab.Query.raise_for_access", lambda _: None) # noqa: PT008 @mock.patch("superset.models.core.Database.get_df") def test_export_results(self, get_df_mock: mock.Mock) -> None: