apache / apache/gravitino

[Bug report] ClickHouse catalog truncates quoted-comma table SETTINGS during load

Open
#12,913 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Java
Stars
3.2k
Forks
935
Avg merge
1d 16h
Merged PRs (30d)
298

Description

### Version

main branch

### Describe what's wrong

The ClickHouse JDBC catalog reads table-level settings from `system.tables.engine_full` and exposes them as `settings.*` table properties, but `ClickHouseTableOperations.parseSettingsClause()` currently splits the SETTINGS clause at every comma.

ClickHouse String settings can legally contain commas. For example, `merge_workload = 'gravitino,quoted,comma'` is preserved completely in `system.tables.engine_full`, but Gravitino loads the corresponding property as only `'gravitino`. A later table recreation therefore cannot preserve the original setting value.

The write path already treats each `settings.*` value as one complete ClickHouse scalar literal, so the problem is limited to readback tokenization.

### Error message and/or stacktrace

No exception or stack trace is produced. The metadata value is silently truncated.

```text
Expected: settings.merge_workload = 'gravitino,quoted,comma'
Actual: settings.merge_workload = 'gravitino
```

### How to reproduce

1. Run Gravitino from the main branch with ClickHouse 24.8.14.
2. Create a native ClickHouse table with a quoted-comma String setting:

```sql
CREATE TABLE test_settings (id Int32)
ENGINE = MergeTree
ORDER BY id
SETTINGS merge_workload = 'gravitino,quoted,comma';
```

3. Query `system.tables.engine_full` and confirm it contains the complete value `'gravitino,quoted,comma'`.
4. Load the table through the Gravitino ClickHouse catalog and inspect its properties.
5. Observe that `settings.merge_workload` is truncated to `'gravitino`.
6. Recreate another table from the loaded setting property and observe that the original scalar literal cannot round-trip correctly.

### Additional context

Issue #11880 and PR #11885 added SETTINGS readback for simple scalar values, but the parser assumes that every comma separates two settings. This issue is a focused parser-correctness follow-up.

The intended fix is limited to splitting on top-level commas and locating the first top-level `=` while ignoring delimiters inside quoted text and nested parentheses. It should preserve the server-returned scalar-literal text, retain existing `settings.*` keys and simple-setting behavior, and fail clearly on structurally malformed metadata without exposing setting values.

The scope does not include ALTER settings from #12768/#12772, column-level SETTINGS, CODEC or TTL parsing, setting-name/type validation, public API changes, or a general ClickHouse SQL parser.

The same truncation was independently reproduced on ClickHouse 25.3.2.39 with `columns_to_prewarm_mark_cache = 'id,name,val'`, confirming that the problem is not specific to the 24.8 project fixture.

Regression coverage should include direct parsing tests for quoted commas, escaping, doubled quotes, nested parentheses, malformed fragments, and duplicate keys, plus one real ClickHouse native-load and load/recreate test.

Contributor guide

Open the contributing guide

Research direction

Begin with ClickHouseTableOperations.parseSettingsClause() and the existing SETTINGS readback added by issues #11880 and #11885. Add direct parsing coverage for quoted commas, escaping, doubled quotes, nested parentheses, malformed fragments, and duplicate keys, plus the real ClickHouse native-load and load/recreate test. Done means complete scalar-literal values round-trip while existing keys and simple-setting behavior remain intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
java, sql
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.