fix: correct escaped single quote parsing in Enum type strings

[chaewonkong]

Summary

Fix incorrect parsing of escaped single quotes in Enum8/Enum16 type strings.

Two bugs existed in extractEnumNamedValues in lib/column/enum.go:

1. skippedValueTokens was never reset between values, causing escape positions from a previous enum
   value to bleed into the next one.
2. When removing multiple backslashes from a single value, positions were not adjusted for prior
   removals, causing off-by-one corruption.

• closes: Bug: parsing escaped enum names with multiple escape sequences #1839

Checklist

Delete items not relevant to your PR:

• Unit and integration tests covering the common scenarios were added

[github-actions]

Summary

This PR fixes two real bugs in extractEnumNamedValues (lib/column/enum.go) that caused corrupted Enum8/Enum16 value names when escaped single quotes were present. Bug 1: skippedValueTokens was never cleared between values, bleeding escape offsets from one value into the next. Bug 2: when stripping multiple backslashes from a single value the removal loop did not compensate for the leftward shift caused by earlier removals. Both fixes are minimal and correct: the reset (skippedValueTokens[:0]) and the skipped-i adjustment solve exactly what is described. The unit tests added verify both scenarios.

Should fix

Missing regression test in tests/issues/issue_1839_test.go.
The project's workflow guidelines (CLAUDE.md) explicitly state: "Regression tests for bug fixes go in tests/issues/ named after the issue number." The unit tests in lib/column/enum_test.go verify the parsing logic in isolation, but there is no integration test that round-trips an Enum8 column whose names contain escaped single quotes through a real ClickHouse instance. An integration test would cover the full stack (type-string received from the server → parsed → inserted/queried) and guard against regressions from future protocol or server changes. Please add tests/issues/issue_1839_test.go with a table-driven test that creates a table with an Enum8 or Enum16 column using names like a'b and a'b'c, inserts a row, and reads it back, for both the native (clickhouse.Open) and database/sql (clickhouse_std) APIs.

Nits

• nit: There is a blank line between the two new table entries in enum_test.go. The rest of the table has no blank lines between entries — remove the extra blank line for consistency.

Verdict

Request changes

[github-actions]

Summary

This PR fixes two bugs in extractEnumNamedValues (lib/column/enum.go) that corrupt enum values when the Enum8/Enum16 type string contains escaped single quotes:

1. Bleeding state: skippedValueTokens was never cleared between enum entries, so backslash positions from earlier values were reapplied when post-processing later values.
2. Off-by-one after multiple removals: When stripping more than one backslash from a single value, positions were not adjusted for the characters already removed, leaving stale bytes in the result.

Both fixes are correct. The adjusted-position formula (skipped-i) accurately accounts for the i elements already deleted. The skippedValueTokens = skippedValueTokens[:0] reset is placed correctly — after the value is committed to values, before the next iteration — and reuses the backing array. The PR includes unit tests and a real-ClickHouse integration test that cover both failure modes.

Should fix

Integration test covers only native TCP; not HTTP or std API (tests/issues/1839_test.go). The bug lives in extractEnumNamedValues, which runs at column-type setup time, so it manifests identically over both protocols and both APIs. Adding an HTTP or database/sql variant would provide end-to-end confidence that the fix propagates through both code paths and satisfies the project checklist. A minimal addition:

func Test1839Std(t *testing.T) {
    testEnv, err := clickhouse_tests.GetTestEnvironment("issues")
    require.NoError(t, err)
    db, err := clickhouse_tests.TestClientWithDefaultSettingsStd(testEnv)
    // ... same table + insert + query pattern
}

Nits

• nit: The two new unit-test cases in enum_test.go use Enum8. The existing suite has Enum8/Enum16 twins for several scenarios (e.g. "Enum8 single zero value" / "Enum16 single zero value"). Adding Enum16 variants for the escaped-quote cases (even if extractEnumNamedValues is shared) keeps the test matrix consistent.

Verdict

Approve

[github-actions]

Summary

This PR fixes two bugs in extractEnumNamedValues (lib/column/enum.go) that caused corrupted Enum8/Enum16 parsing when type strings contain escaped single quotes. Bug 1: skippedValueTokens was never cleared between enum values, so escape positions from one value bled into the next. Bug 2: when stripping multiple backslashes from a single value, positions were not adjusted for earlier removals, producing off-by-one corruption. Both fixes are minimal, correct, and well-targeted. The unit tests in enum_test.go directly cover the broken cases, and the integration test in tests/issues/1839_test.go verifies the full round-trip. The approach is sound.

Should fix

• Protocol coverage — extractEnumNamedValues runs whenever ClickHouse sends a column type string, which happens on both the native TCP and HTTP paths. The integration test only exercises clickhouse.Native. Please add an HTTP-protocol sub-test, following the pattern in tests/issues/1801_test.go:

  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)
          // ...
      })
  }

Nits

• nit: Import groups in tests/issues/1839_test.go combine github.com/stretchr/testify/require (external) and clickhouse_tests "github.com/ClickHouse/clickhouse-go/v2/tests" (internal) into one block. The project convention (see tests/issues/1801_test.go) is three separate groups — stdlib → external → internal — each separated by a blank line.

Verdict

Approve — the core logic fixes are correct and the unit-test coverage is solid. Addressing protocol coverage in the integration test before merge would be ideal.

[chaewonkong]

The CI failure in test (1.25, 25.10) seems unrelated to this PR. Test1229 is flaky and fails intermittently on main.