- 
                Notifications
    You must be signed in to change notification settings 
- Fork 748
sql mode - validate for duckdb #6460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| The latest updates on your projects. Learn more about Vercel for GitHub. 
 | 
| Breaking changes detected in the OpenAPI specification! | 
78b784f    to
    e4ceb06      
    Compare
  
    | Breaking changes detected in the OpenAPI specification! | 
| Breaking changes detected in the OpenAPI specification! | 
| Breaking changes detected in the OpenAPI specification! | 
| Breaking changes detected in the OpenAPI specification! | 
| </div> | ||
| </div> | ||
| {sqlValidationError && ( | ||
| <SqlValidationError error={sqlValidationError} /> | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in order to avoid an additional re-render of this component, you could make this <CellSqlValidationError cellId={cellId}/> which uses <SqlValidationError
| Breaking changes detected in the OpenAPI specification! | 
| Breaking changes detected in the OpenAPI specification! | 
9d68812    to
    eb0187d      
    Compare
  
    | Breaking changes detected in the OpenAPI specification! | 
| Breaking changes detected in the OpenAPI specification! | 
| Breaking changes detected in the OpenAPI specification! | 
| Todo in next PRs: 
 | 
| Breaking changes detected in the OpenAPI specification! | 
    
      
        1 similar comment
      
    
  
    | Breaking changes detected in the OpenAPI specification! | 
| } | ||
|  | ||
| function sqlValidationExtension(): Extension { | ||
| let debounceTimeout: NodeJS.Timeout | null = null; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be int. if the type give you a hard time, you can change setTimeout to window.setTimeout and window.clearTimeout
| } catch (error) { | ||
| Logger.warn("Failed to validate SQL", { error }); | ||
| } | ||
| }, 300); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe pull up to the top
| request_id=request.request_id, | ||
| result=None, # We aren't using the result yet | ||
| error=error, | ||
| ).broadcast() | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we add tests for this code path? maybe un test_runtime.py
📝 Summary
This runs queries in EXPLAIN mode on keypress. It's only enabled when feat flag is on & using Duckdb engine.
The feat flag is not exposed.
CleanShot.2025-09-24.at.01.01.54.mp4
🔍 Description of Changes
📋 Checklist