Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,36 @@ var truncateFormat = regexp.MustCompile(`(?i)\sFORMAT\s+[^\s]+`)
var truncateValues = regexp.MustCompile(`\sVALUES\s.*$`)
var extractInsertColumnsMatch = regexp.MustCompile(`(?si)INSERT INTO .+\s\((?P<Columns>.+)\)$`)

// extractInsertSettingsMatch captures a trailing SETTINGS clause. The `\w+\s*=`
// after the SETTINGS keyword requires an actual `name = value` assignment so a table
// or column merely named "settings" is not mistaken for a settings clause.
var extractInsertSettingsMatch = regexp.MustCompile(`(?i)\s+(SETTINGS\s+\w+\s*=.+?)(?:\s+VALUES)?\s*$`)
Comment thread
polyglotAI-bot marked this conversation as resolved.
Outdated

func extractNormalizedInsertQueryAndColumns(query string) (normalizedQuery string, tableName string, columns []string, err error) {
query = truncateFormat.ReplaceAllString(query, "")
query = truncateValues.ReplaceAllString(query, "")

// A SETTINGS clause may follow the optional column list, e.g.
// "INSERT INTO t (a, b) SETTINGS async_insert=1". Capture it so it is preserved in
// the normalized query sent to the server, and strip it from the query before the
// table name and columns are extracted so it does not leak into either.
var settingsClause string
if loc := extractInsertSettingsMatch.FindStringSubmatchIndex(query); loc != nil {
settingsClause = strings.TrimSpace(query[loc[2]:loc[3]])
query = query[:loc[0]]
}

matches := normalizeInsertQueryMatch.FindStringSubmatch(query)
if len(matches) == 0 {
err = fmt.Errorf("invalid INSERT query: %s", query)
return
}

normalizedQuery = fmt.Sprintf("%s FORMAT Native", matches[1])
normalizedQuery = matches[1]
if settingsClause != "" {
normalizedQuery += " " + settingsClause
}
normalizedQuery += " FORMAT Native"
tableName = strings.TrimSpace(matches[2])

columns = make([]string, 0)
Expand Down
78 changes: 78 additions & 0 deletions batch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,84 @@ INSERT INTO ` + "`_test_1345# $.ДБ`.`2. Таблица №2`" + ` (col1, col2)
expectedColumns: []string{"col1", "col2"},
expectedError: false,
},
{
name: "Insert with SETTINGS after column list",
query: "INSERT INTO table_name (col1, col2) SETTINGS async_insert=1",
expectedNormalizedQuery: "INSERT INTO table_name (col1, col2) SETTINGS async_insert=1 FORMAT Native",
expectedTableName: "table_name",
expectedColumns: []string{"col1", "col2"},
expectedError: false,
},
Comment thread
polyglotAI-bot marked this conversation as resolved.
{
name: "Insert with SETTINGS after column list and trailing VALUES keyword",
query: "INSERT INTO table_name (col1, col2) SETTINGS async_insert=1 VALUES",
expectedNormalizedQuery: "INSERT INTO table_name (col1, col2) SETTINGS async_insert=1 FORMAT Native",
expectedTableName: "table_name",
expectedColumns: []string{"col1", "col2"},
expectedError: false,
},
{
name: "Insert with multiple SETTINGS after column list and VALUES tuples",
query: "INSERT INTO table_name (col1, col2) SETTINGS async_insert=1, wait_for_async_insert=0 VALUES (1, 2)",
expectedNormalizedQuery: "INSERT INTO table_name (col1, col2) SETTINGS async_insert=1, wait_for_async_insert=0 FORMAT Native",
expectedTableName: "table_name",
expectedColumns: []string{"col1", "col2"},
expectedError: false,
},
{
name: "Insert with SETTINGS and no column list",
query: "INSERT INTO table_name SETTINGS async_insert=1",
expectedNormalizedQuery: "INSERT INTO table_name SETTINGS async_insert=1 FORMAT Native",
expectedTableName: "table_name",
expectedColumns: []string{},
expectedError: false,
},
{
name: "Insert with lowercase settings keyword after column list",
query: "INSERT INTO table_name (col1, col2) settings async_insert=1 VALUES (1, 2)",
expectedNormalizedQuery: "INSERT INTO table_name (col1, col2) settings async_insert=1 FORMAT Native",
expectedTableName: "table_name",
expectedColumns: []string{"col1", "col2"},
expectedError: false,
},
{
name: "Insert with SETTINGS after column list and explicit FORMAT",
query: "INSERT INTO table_name (col1, col2) SETTINGS async_insert=1 FORMAT JSONEachRow",
expectedNormalizedQuery: "INSERT INTO table_name (col1, col2) SETTINGS async_insert=1 FORMAT Native",
expectedTableName: "table_name",
expectedColumns: []string{"col1", "col2"},
expectedError: false,
},
{
name: "Insert with column named settings is not treated as a SETTINGS clause",
query: "INSERT INTO table_name (col1, settings) VALUES (1, 2)",
expectedNormalizedQuery: "INSERT INTO table_name (col1, settings) FORMAT Native",
expectedTableName: "table_name",
expectedColumns: []string{"col1", "settings"},
expectedError: false,
},
{
name: "Insert into table named settings is not treated as a SETTINGS clause",
query: "INSERT INTO settings (col1, col2) VALUES (1, 2)",
expectedNormalizedQuery: "INSERT INTO settings (col1, col2) FORMAT Native",
expectedTableName: "settings",
expectedColumns: []string{"col1", "col2"},
expectedError: false,
},
{
name: "Insert with multiline column list ending in a column named settings",
query: `INSERT INTO table_name (
col1,
settings
) VALUES (1, 2)`,
expectedNormalizedQuery: `INSERT INTO table_name (
col1,
settings
) FORMAT Native`,
expectedTableName: "table_name",
expectedColumns: []string{"col1", "settings"},
expectedError: false,
},
}

for _, tc := range testCases {
Expand Down
72 changes: 72 additions & 0 deletions tests/issues/1919_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package issues

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/require"

"github.com/ClickHouse/clickhouse-go/v2"
clickhouse_tests "github.com/ClickHouse/clickhouse-go/v2/tests"
)

// Test1919 verifies that PrepareBatch preserves an inline SETTINGS clause placed after
// the column list instead of silently dropping it from the query sent to the server.
// See https://github.com/ClickHouse/clickhouse-go/issues/1919.
func Test1919(t *testing.T) {
testEnv, err := clickhouse_tests.GetTestEnvironment("issues")
require.NoError(t, err)

for _, protocol := range []clickhouse.Protocol{clickhouse.Native, clickhouse.HTTP} {
t.Run(fmt.Sprintf("%v", protocol), func(t *testing.T) {
conn, err := clickhouse_tests.GetConnection("issues", t, protocol, clickhouse_tests.TestClientDefaultSettings(testEnv), nil, nil)
require.NoError(t, err)
t.Cleanup(func() { conn.Close() })

tableName := fmt.Sprintf("test_1919_%v", protocol)
require.NoError(t, conn.Exec(context.Background(), fmt.Sprintf(`CREATE TABLE %s
(
col1 UInt64,
col2 String
)
ENGINE = MergeTree
ORDER BY col1;`, tableName)), "Create table failed")
t.Cleanup(func() {
if err := conn.Exec(context.Background(), fmt.Sprintf("DROP TABLE IF EXISTS %s", tableName)); err != nil {
t.Logf("DROP TABLE %s failed: %v", tableName, err)
}
})

// A valid SETTINGS clause after the column list must be accepted and the
// rows inserted. Before the fix the clause was dropped from the normalized
// query, so this only exercises the happy path staying intact.
batch, err := conn.PrepareBatch(context.Background(), fmt.Sprintf("INSERT INTO %s (col1, col2) SETTINGS async_insert=0", tableName))
require.NoError(t, err, "PrepareBatch with SETTINGS after column list failed")
for i := range 10 {
require.NoError(t, batch.Append(uint64(i), "value"))
}
require.NoError(t, batch.Send())

var count uint64
require.NoError(t, conn.QueryRow(context.Background(), fmt.Sprintf("SELECT count() FROM %s", tableName)).Scan(&count))
require.Equal(t, uint64(10), count)

// The SETTINGS clause must actually reach the server: an unknown setting has
// to surface as an error rather than being silently dropped. Before the fix
// the clause was stripped, so the insert succeeded and no error was raised.
send := func(query string) error {
b, err := conn.PrepareBatch(context.Background(), query)
if err != nil {
return err
}
if err = b.Append(uint64(1), "value"); err != nil {
return err
}
return b.Send()
}
err = send(fmt.Sprintf("INSERT INTO %s (col1, col2) SETTINGS nonexistent_setting_for_test_1919=1", tableName))
require.ErrorContains(t, err, "nonexistent_setting_for_test_1919")
})
}
}
Loading