Skip to content

Fix: Support readonly=1 users by conditionally setting query settings#471

Open
guoyi8 wants to merge 1 commit intohousepower:masterfrom
guoyi8:fix/readonly-users-conditional-settings
Open

Fix: Support readonly=1 users by conditionally setting query settings#471
guoyi8 wants to merge 1 commit intohousepower:masterfrom
guoyi8:fix/readonly-users-conditional-settings

Conversation

@guoyi8
Copy link

@guoyi8 guoyi8 commented Feb 1, 2026

Problem:
Users with readonly=1 cannot use clickhouse-native-jdbc because the driver unconditionally sets max_result_rows and result_overflow_mode settings, which violates the readonly=1 restriction.

Solution:
Only set these settings when maxRows > 0 (explicitly requested). This allows readonly=1 users to use the driver while preserving setMaxRows() functionality for other users.

Benefits:

  • ✅ Supports readonly=1 users (recommended production config)
  • ✅ Maintains backward compatibility
  • ✅ Minimal code change (5 lines)
  • ✅ Follows principle: "don't set what you don't need"

Tested:

  • ✅ Works with readonly=1 users (without setMaxRows)
  • ✅ Works with SQL LIMIT (recommended approach)
  • ✅ setMaxRows() still works for readonly=0 and readonly=2

Related issues:

Problem:
Users with readonly=1 cannot use clickhouse-native-jdbc because the driver
unconditionally sets max_result_rows and result_overflow_mode settings,
which violates the readonly=1 restriction.

Solution:
Only set these settings when maxRows > 0 (explicitly requested).
This allows readonly=1 users to use the driver while preserving
setMaxRows() functionality for other users.

Benefits:
- ✅ Supports readonly=1 users (recommended production config)
- ✅ Maintains backward compatibility
- ✅ Minimal code change (5 lines)
- ✅ Follows principle: "don't set what you don't need"

Tested:
- ✅ Works with readonly=1 users (without setMaxRows)
- ✅ Works with SQL LIMIT (recommended approach)
- ✅ setMaxRows() still works for readonly=0 and readonly=2

Related issues:
- ClickHouse/clickhouse-java#324
- ClickHouse/clickhouse-java#2441
- grafana/clickhouse-datasource#325
- dbeaver#38280
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.

1 participant