Skip to content

Conversation

zaleslaw
Copy link
Collaborator

Fixes #1326

Also I investigated over popular approaches in the supported dialects

To avoid this validation, we have a flag which could be recommended for the user with custom database

@zaleslaw zaleslaw requested review from Copilot and koperagen July 16, 2025 15:35
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for validating and executing SQL queries that start with WITH, alongside existing SELECT queries, and includes a new integration test using the WITH clause.

  • Extend SQL validation to allow queries beginning with WITH, VALUES, or TABLE
  • Introduce ALLOWED_SQL_OPERATORS and update validation logic
  • Add a test case to ensure DataFrame.readSqlQuery handles WITH clauses correctly

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/postgresH2Test.kt Refactored SQL string trimming and added a new WITH-clause test
dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/readJdbc.kt Added ALLOWED_SQL_OPERATORS and updated query-start validation logic
Comments suppressed due to low confidence (4)

dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/postgresH2Test.kt:380

  • The test verifies the row count and names of high earners, but it doesn't assert the corresponding salary column values. Adding assertions for the salary values will ensure both columns are validated.
            resultDataFrame[0][0] shouldBe "Alice"

dataframe-jdbc/src/main/kotlin/org/jetbrains/kotlinx/dataframe/io/readJdbc.kt:566

  • This documentation is outdated as the validation now allows queries starting with WITH, VALUES, and TABLE in addition to SELECT. Please update the doc comment to reflect the updated allowed operators list.
 * Validates if the SQL query is safe and starts with SELECT.

dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/postgresH2Test.kt:104

  • The SQL string assigned to createTableStatement is already trimmed using trimIndent(). The additional .trimIndent() call on execution is redundant and can be removed to simplify the code.
            connection.createStatement().execute(createTableStatement.trimIndent())

dataframe-jdbc/src/test/kotlin/org/jetbrains/kotlinx/dataframe/io/h2/postgresH2Test.kt:125

  • Similarly, createTableQuery is trimmed at assignment, making the .trimIndent() call here redundant. Consider removing it to avoid unnecessary repetition.
            connection.createStatement().execute(createTableQuery.trimIndent())

@zaleslaw zaleslaw merged commit 1053072 into master Jul 16, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CTE(common table expression) support for query

2 participants