korbit-ai[bot] commented on code in PR #33340:
URL: https://github.com/apache/superset/pull/33340#discussion_r2080105266


##########
superset-frontend/src/database/reducers.ts:
##########
@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import type { QueryAdhocState } from './types';
+
+const initialState: QueryAdhocState = {
+  isLoading: null,

Review Comment:
   ### Incorrect Loading State Type <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The 'isLoading' state is initialized as null when it should be a boolean to 
properly represent loading state.
   
   ###### Why this matters
   Using null for a loading state can lead to ambiguous state handling and 
potentially incorrect UI rendering decisions.
   
   ###### Suggested change ∙ *Feature Preview*
   Initialize isLoading as a boolean false:
   ```typescript
   const initialState: QueryAdhocState = {
     isLoading: false,
     sql: null,
     queryResult: null,
     error: null,
   };
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c9a79524-66e0-4211-bf35-c6a29697b45c/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c9a79524-66e0-4211-bf35-c6a29697b45c?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c9a79524-66e0-4211-bf35-c6a29697b45c?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c9a79524-66e0-4211-bf35-c6a29697b45c?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/c9a79524-66e0-4211-bf35-c6a29697b45c)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:134e577c-d504-4777-837c-7cdda6a17ea9 -->
   
   
   [](134e577c-d504-4777-837c-7cdda6a17ea9)



##########
superset-frontend/src/database/actions.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { makeApi } from '@superset-ui/core';
+import { ThunkDispatch } from 'redux-thunk';
+import { AnyAction } from 'redux';
+import { QueryExecutePayload, QueryExecuteResponse } from './types';
+
+export const executeQueryApi = makeApi<
+  QueryExecutePayload,
+  QueryExecuteResponse
+>({
+  method: 'POST',
+  endpoint: '/api/v1/sqllab/execute',
+});
+
+export function setQueryIsLoading(isLoading: boolean) {
+  return {
+    type: 'SET_QUERY_IS_LOADING',
+    payload: isLoading,
+  };
+}
+export function setQueryResult(queryResult: QueryExecuteResponse) {
+  return {
+    type: 'SET_QUERY_RESULT',
+    payload: queryResult,
+  };
+}
+export function resetDatabaseState() {
+  return {
+    type: 'RESET_DATABASE_STATE',
+  };
+}
+export function setQueryError(error: string) {
+  return {
+    type: 'SET_QUERY_ERROR',
+    payload: error,
+  };
+}
+export function executeQuery(payload: QueryExecutePayload) {
+  return async function (dispatch: ThunkDispatch<any, undefined, AnyAction>) {
+    try {
+      dispatch(setQueryIsLoading(true));
+      const result = await executeQueryApi(payload);
+      dispatch(setQueryResult(result as QueryExecuteResponse));

Review Comment:
   ### Unsafe API Response Type Assertion <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The type assertion (as QueryExecuteResponse) bypasses TypeScript's type 
checking without validating the actual response structure.
   
   ###### Why this matters
   If the API response structure changes or is malformed, the application might 
crash when trying to access properties that don't exist.
   
   ###### Suggested change ∙ *Feature Preview*
   Implement runtime validation of the API response:
   ```typescript
   const result = await executeQueryApi(payload);
   if (!isValidQueryResponse(result)) {
     throw new Error('Invalid response format from server');
   }
   dispatch(setQueryResult(result));
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2c188fa0-22f5-4a12-a693-fd641391133e/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2c188fa0-22f5-4a12-a693-fd641391133e?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2c188fa0-22f5-4a12-a693-fd641391133e?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2c188fa0-22f5-4a12-a693-fd641391133e?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2c188fa0-22f5-4a12-a693-fd641391133e)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:a53c4f13-0757-4d45-b7f1-56f01a30c94f -->
   
   
   [](a53c4f13-0757-4d45-b7f1-56f01a30c94f)



##########
superset-frontend/src/components/Datasource/Field.tsx:
##########
@@ -62,32 +66,51 @@
     onChange: onControlChange,
   });
   return (
-    <FormItem
-      label={
-        <FormLabel className="m-r-5">
-          {label || fieldKey}
-          {compact && description && (
-            <Tooltip id="field-descr" placement="right" title={description}>
-              {/* TODO: Remove fa-icon */}
-              {/* eslint-disable-next-line icons/no-fa-icons-usage */}
-              <i className="fa fa-info-circle m-l-5" />
-            </Tooltip>
-          )}
-        </FormLabel>
+    <div
+      css={
+        additionalControl &&
+        css`
+          position: relative;
+        `
       }
-      css={inline && formItemInlineCss}
     >
-      {hookedControl}
-      {!compact && description && (
+      {additionalControl}
+      <FormItem
+        label={
+          <FormLabel className="m-r-5">
+            {label || fieldKey}
+            {compact && description && (
+              <Tooltip id="field-descr" placement="right" title={description}>
+                <Icons.InfoCircleFilled iconSize="s" className="m-l-5" />
+              </Tooltip>
+            )}
+          </FormLabel>
+        }
+        css={inline && formItemInlineCss}
+      >
+        {hookedControl}
+        {!compact && description && (
+          <div
+            css={(theme: SupersetTheme) => ({
+              color: theme.colors.grayscale.base,
+              [inline ? 'marginLeft' : 'marginTop']: theme.gridUnit,
+            })}
+          >
+            {description}
+          </div>
+        )}
+      </FormItem>
+      {errorMessage && (
         <div
           css={(theme: SupersetTheme) => ({
-            color: theme.colors.grayscale.base,
-            [inline ? 'marginLeft' : 'marginTop']: theme.gridUnit,
+            color: theme.colors.error.base,
+            marginTop: -16,

Review Comment:
   ### Hardcoded Magic Number in Styling <sub>![category 
Readability](https://img.shields.io/badge/Readability-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Magic number -16 is used for margin styling without explanation or variable 
reference.
   
   ###### Why this matters
   Hardcoded numbers make it difficult to understand the intent and 
relationship to other styling values, and complicate maintenance.
   
   ###### Suggested change ∙ *Feature Preview*
   ```typescript
   marginTop: theme.gridUnit * -4, // -16px for error message spacing
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4ea690f1-22e5-4564-8a8f-817f5e435b55/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4ea690f1-22e5-4564-8a8f-817f5e435b55?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4ea690f1-22e5-4564-8a8f-817f5e435b55?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4ea690f1-22e5-4564-8a8f-817f5e435b55?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4ea690f1-22e5-4564-8a8f-817f5e435b55)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:8fc3968e-3b6d-4c69-bffe-22192900bcdc -->
   
   
   [](8fc3968e-3b6d-4c69-bffe-22192900bcdc)



##########
superset-frontend/src/database/actions.ts:
##########
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { makeApi } from '@superset-ui/core';
+import { ThunkDispatch } from 'redux-thunk';
+import { AnyAction } from 'redux';
+import { QueryExecutePayload, QueryExecuteResponse } from './types';
+
+export const executeQueryApi = makeApi<
+  QueryExecutePayload,
+  QueryExecuteResponse
+>({
+  method: 'POST',
+  endpoint: '/api/v1/sqllab/execute',
+});
+
+export function setQueryIsLoading(isLoading: boolean) {
+  return {
+    type: 'SET_QUERY_IS_LOADING',
+    payload: isLoading,
+  };
+}
+export function setQueryResult(queryResult: QueryExecuteResponse) {
+  return {
+    type: 'SET_QUERY_RESULT',
+    payload: queryResult,
+  };
+}
+export function resetDatabaseState() {
+  return {
+    type: 'RESET_DATABASE_STATE',
+  };
+}
+export function setQueryError(error: string) {
+  return {
+    type: 'SET_QUERY_ERROR',
+    payload: error,
+  };
+}
+export function executeQuery(payload: QueryExecutePayload) {
+  return async function (dispatch: ThunkDispatch<any, undefined, AnyAction>) {
+    try {
+      dispatch(setQueryIsLoading(true));
+      const result = await executeQueryApi(payload);
+      dispatch(setQueryResult(result as QueryExecuteResponse));

Review Comment:
   ### Missing Query Rate Limiting <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   No debounce/throttle mechanism implemented for SQL query execution, which 
could lead to rapid-fire API calls if the user makes multiple quick changes.
   
   ###### Why this matters
   Without rate limiting, users could accidentally trigger multiple expensive 
SQL queries in quick succession, potentially overwhelming the server and 
degrading application performance.
   
   ###### Suggested change ∙ *Feature Preview*
   Implement debouncing using a utility like lodash's debounce:
   ```typescript
   import { debounce } from 'lodash';
   
   export const debouncedExecuteQuery = debounce(
     (payload: QueryExecutePayload, dispatch: ThunkDispatch<any, undefined, 
AnyAction>) => {
       executeQuery(payload)(dispatch);
     },
     500
   );
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/18cd9755-f86a-4e0e-96ee-75661d7679cd/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/18cd9755-f86a-4e0e-96ee-75661d7679cd?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/18cd9755-f86a-4e0e-96ee-75661d7679cd?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/18cd9755-f86a-4e0e-96ee-75661d7679cd?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/18cd9755-f86a-4e0e-96ee-75661d7679cd)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:315bc01f-1e47-4ea7-8f4e-7ffb7f10f2e9 -->
   
   
   [](315bc01f-1e47-4ea7-8f4e-7ffb7f10f2e9)



##########
superset-frontend/src/database/types.ts:
##########
@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+export interface QueryExecutePayload {
+  client_id: string;
+  database_id: number;
+  json: boolean;
+  runAsync: boolean;
+  catalog: string | null;
+  schema: string;
+  sql: string;
+  tmp_table_name: string;
+  select_as_cta: boolean;
+  ctas_method: string;
+  queryLimit: number;
+  expand_data: boolean;
+}
+export interface Column {
+  name: string;
+  type: string;
+  is_dttm: boolean;
+  type_generic: number;
+  is_hidden: boolean;
+  column_name: string;
+}

Review Comment:
   ### Redundant Column Properties <sub>![category 
Design](https://img.shields.io/badge/Design-0d9488)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The Column interface has redundant properties 'name' and 'column_name' which 
appear to represent the same concept.
   
   ###### Why this matters
   Having duplicate properties with different names increases maintenance 
burden and can lead to inconsistent usage across the codebase. It violates the 
DRY principle and can cause confusion about which property should be used.
   
   ###### Suggested change ∙ *Feature Preview*
   ```typescript
   export interface Column {
     name: string;
     type: string;
     is_dttm: boolean;
     type_generic: number;
     is_hidden: boolean;
   }
   ```
   Standardize on using either 'name' or 'column_name', not both.
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/317b2288-5434-431c-afc8-4c0b20ce8553/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/317b2288-5434-431c-afc8-4c0b20ce8553?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/317b2288-5434-431c-afc8-4c0b20ce8553?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/317b2288-5434-431c-afc8-4c0b20ce8553?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/317b2288-5434-431c-afc8-4c0b20ce8553)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:d9c92136-a816-4f6d-8023-a899eedb7d61 -->
   
   
   [](d9c92136-a816-4f6d-8023-a899eedb7d61)



##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -1078,14 +1103,62 @@
                       <TextAreaControl
                         language="sql"
                         offerEditInModal={false}
-                        minLines={20}
+                        minLines={10}
                         maxLines={Infinity}
                         readOnly={!this.state.isEditMode}
                         resize="both"
                         tooltipOptions={sqlTooltipOptions}
                       />
                     }
+                    additionalControl={
+                      <div
+                        css={css`
+                          position: absolute;
+                          right: 0;
+                          top: 0;
+                          z-index: 2;
+                        `}
+                      >
+                        <Button
+                          css={css`
+                            align-self: flex-end;
+                            height: 24px;
+                            padding-left: 6px;
+                            padding-right: 6px;
+                          `}
+                          size="small"
+                          buttonStyle="primary"
+                          onClick={() => {
+                            this.onQueryRun();
+                          }}
+                        >
+                          <Icons.CaretRightFilled
+                            iconSize="s"
+                            css={theme => ({
+                              color: theme.colors.grayscale.light5,
+                            })}
+                          />
+                        </Button>
+                      </div>
+                    }
+                    errorMessage={
+                      this.props.database?.error && t('Error executing query.')
+                    }
                   />
+                  {this.props.database?.queryResult && (
+                    <ResultTable
+                      data={this.props.database.queryResult.data}
+                      queryId={this.props.database.queryResult.query.id}
+                      
orderedColumnKeys={this.props.database.queryResult.columns.map(
+                        col => col.column_name,
+                      )}
+                      height={100}

Review Comment:
   ### Insufficient result table height <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The ResultTable has a fixed height of 100px which may not be sufficient for 
displaying query results effectively.
   
   ###### Why this matters
   A fixed small height could make it difficult for users to view and analyze 
query results, especially for queries returning many rows.
   
   ###### Suggested change ∙ *Feature Preview*
   Make the height dynamic based on content or viewport:
   ```javascript
   height={Math.min(window.innerHeight * 0.4, 400)} // 40% of viewport height, 
max 400px
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e0c80c17-a0f6-41c5-bc94-1f0c52c2076e/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e0c80c17-a0f6-41c5-bc94-1f0c52c2076e?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e0c80c17-a0f6-41c5-bc94-1f0c52c2076e?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e0c80c17-a0f6-41c5-bc94-1f0c52c2076e?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e0c80c17-a0f6-41c5-bc94-1f0c52c2076e)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:958ee215-bd01-459f-a2ba-0c5af8d23ccf -->
   
   
   [](958ee215-bd01-459f-a2ba-0c5af8d23ccf)



##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -698,6 +706,23 @@ class DatasourceEditor extends PureComponent {
     this.validate(this.onChange);
   }
 
+  async onQueryRun() {
+    this.props.runQuery({
+      client_id: this.props.clientId,
+      database_id: this.state.datasource.database.id,
+      json: true,
+      runAsync: false,
+      catalog: this.state.datasource.catalog,
+      schema: this.state.datasource.schema,
+      sql: this.state.datasource.sql,
+      tmp_table_name: '',
+      select_as_cta: false,
+      ctas_method: 'TABLE',
+      queryLimit: 25,
+      expand_data: true,
+    });

Review Comment:
   ### Missing required clientId prop <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The client_id is being accessed from props but isn't defined in the 
component's propTypes or being passed from the parent component.
   
   ###### Why this matters
   This could lead to undefined client_id in the query execution, potentially 
causing the query to fail or be tracked incorrectly.
   
   ###### Suggested change ∙ *Feature Preview*
   Add clientId to propTypes and ensure it's passed from the parent component:
   ```javascript
   const propTypes = {
     clientId: PropTypes.string.isRequired,
     // ... other propTypes
   };
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f44ab94d-6601-4253-8f69-eaa181b10fb3/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f44ab94d-6601-4253-8f69-eaa181b10fb3?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f44ab94d-6601-4253-8f69-eaa181b10fb3?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f44ab94d-6601-4253-8f69-eaa181b10fb3?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/f44ab94d-6601-4253-8f69-eaa181b10fb3)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:4a695d74-8d83-4731-b971-bc29457c988a -->
   
   
   [](4a695d74-8d83-4731-b971-bc29457c988a)



##########
superset-frontend/src/components/Datasource/DatasourceEditor.jsx:
##########
@@ -698,6 +706,23 @@
     this.validate(this.onChange);
   }
 
+  async onQueryRun() {
+    this.props.runQuery({
+      client_id: this.props.clientId,
+      database_id: this.state.datasource.database.id,
+      json: true,
+      runAsync: false,
+      catalog: this.state.datasource.catalog,
+      schema: this.state.datasource.schema,
+      sql: this.state.datasource.sql,
+      tmp_table_name: '',
+      select_as_cta: false,
+      ctas_method: 'TABLE',
+      queryLimit: 25,
+      expand_data: true,
+    });
+  }

Review Comment:
   ### Synchronous Query Execution <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The query execution is set to synchronous mode (runAsync: false) which can 
block the UI thread for long-running queries.
   
   ###### Why this matters
   Synchronous query execution can lead to poor user experience as the UI 
becomes unresponsive while waiting for query results.
   
   ###### Suggested change ∙ *Feature Preview*
   Set `runAsync: true` and implement proper loading states and error handling:
   ```jsx
   async onQueryRun() {
     this.setState({ isLoading: true });
     try {
       await this.props.runQuery({
         ...,
         runAsync: true,
       });
     } catch (error) {
       // Handle error
     } finally {
       this.setState({ isLoading: false });
     }
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bd11079d-8fb7-418c-aa71-ecb06fc27323/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bd11079d-8fb7-418c-aa71-ecb06fc27323?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bd11079d-8fb7-418c-aa71-ecb06fc27323?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bd11079d-8fb7-418c-aa71-ecb06fc27323?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bd11079d-8fb7-418c-aa71-ecb06fc27323)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:f8a558d1-0efa-461b-9b05-a1dfb2740c85 -->
   
   
   [](f8a558d1-0efa-461b-9b05-a1dfb2740c85)



##########
superset-frontend/src/components/Datasource/Field.tsx:
##########
@@ -21,23 +21,25 @@ import { useCallback, ReactNode, ReactElement, cloneElement 
} from 'react';
 import { css, SupersetTheme } from '@superset-ui/core';
 import { Tooltip } from 'src/components/Tooltip';
 import { FormItem, FormLabel } from 'src/components/Form';
+import { Icons } from 'src/components/Icons';
 
 const formItemInlineCss = css`
   .ant-form-item-control-input-content {
     display: flex;
     flex-direction: row;
   }
 `;
-
 interface FieldProps<V> {
   fieldKey: string;
   value?: V;
   label: string;
   description?: ReactNode;
   control: ReactElement;
+  additionalControl?: ReactElement;
   onChange: (fieldKey: string, newValue: V) => void;
   compact: boolean;
   inline: boolean;
+  errorMessage?: string;
 }

Review Comment:
   ### Missing interface property descriptions <sub>![category 
Documentation](https://img.shields.io/badge/Documentation-7c3aed)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The interface properties lack descriptions explaining their purpose and 
usage implications
   
   ###### Why this matters
   This makes it harder for developers to understand how to properly use these 
props, potentially leading to incorrect implementation or requiring them to 
dive into the implementation details
   
   ###### Suggested change ∙ *Feature Preview*
   ```typescript
   interface FieldProps<V> {
     /** Unique identifier for the field */
     fieldKey: string;
     /** Current value of the field */
     value?: V;
     /** Display label for the field */
     label: string;
     /** Optional help text shown below the field (or in tooltip if 
compact=true) */
     description?: ReactNode;
     /** The form control element to render */
     control: ReactElement;
     /** Optional secondary control element */
     additionalControl?: ReactElement;
     /** Callback when field value changes */
     onChange: (fieldKey: string, newValue: V) => void;
     /** Whether to show description in tooltip instead of below field */
     compact: boolean;
     /** Whether to render the field inline */
     inline: boolean;
     /** Error message to display below the field */
     errorMessage?: string;
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/42f329c3-98f4-4df6-8c5d-b67bdb878d96/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/42f329c3-98f4-4df6-8c5d-b67bdb878d96?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/42f329c3-98f4-4df6-8c5d-b67bdb878d96?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/42f329c3-98f4-4df6-8c5d-b67bdb878d96?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/42f329c3-98f4-4df6-8c5d-b67bdb878d96)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:f2e66c1c-875d-4567-ace5-9617dd31652b -->
   
   
   [](f2e66c1c-875d-4567-ace5-9617dd31652b)



-- 
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]

Reply via email to