Skip to content

Add adhoc option to control prepared-statement caching#35

Open
pimpin wants to merge 1 commit into
pimpin/n1ql-positional-params-v2from
pimpin/n1ql-adhoc-option
Open

Add adhoc option to control prepared-statement caching#35
pimpin wants to merge 1 commit into
pimpin/n1ql-positional-params-v2from
pimpin/n1ql-adhoc-option

Conversation

@pimpin

@pimpin pimpin commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add adhoc: true to N1ql.config default, preserving current behavior (queries are not cached)
  • Forward the adhoc option to Couchbase::Options::Query in both n1ql.rb (run_query) and relation.rb (build_query_options)
  • Include adhoc value in debug log messages

Depends on: #34 (Parameterize N1QL queries)

Context

With parameterized queries in place (PR #34), setting adhoc: false enables Couchbase to cache query plans for repeated queries. This PR exposes the option without changing the default behavior.

Users can opt-in to prepared-statement caching via:

CouchbaseOrm::N1ql.config({ adhoc: false })

Test plan

  • Existing log specs updated to include adhoc: true in expected messages
  • /test --fail-fast=false

🤖 Generated with Claude Code

Default adhoc to true in N1ql.config to preserve current behavior
(queries are not cached). When set to false, Couchbase will cache the
parameterized query plan, improving performance for repeated queries.
The option is forwarded to Couchbase::Options::Query in both n1ql and
relation query paths.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for the adhoc query option in Couchbase N1QL queries, defaulting it to true in the configuration and passing it through query options and logging. The review feedback highlights an issue in CouchbaseOrm::N1ql.config where setting a partial configuration overwrites the entire thread-local configuration, causing other default options to be lost. A merge strategy is suggested to preserve default values when only a subset of options is configured.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread lib/couchbase-orm/n1ql.rb
Comment on lines 23 to 29
def self.config(new_config = nil)
Thread.current['__couchbaseorm_n1ql_config__'] = new_config if new_config
Thread.current['__couchbaseorm_n1ql_config__'] || {
scan_consistency: DEFAULT_SCAN_CONSISTENCY
scan_consistency: DEFAULT_SCAN_CONSISTENCY,
adhoc: true
}
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Currently, if a user configures a single option (e.g., CouchbaseOrm::N1ql.config({ adhoc: false })), the entire thread-local configuration hash is overwritten. This causes other configuration options (like scan_consistency) to be lost and default to nil instead of their default values.

To prevent partial configurations from wiping out other defaults, we should merge the new configuration into the existing thread-local configuration, and always merge the thread-local configuration on top of the default configuration hash.

        def self.config(new_config = nil)
            if new_config
                Thread.current['__couchbaseorm_n1ql_config__'] = (Thread.current['__couchbaseorm_n1ql_config__'] || {}).merge(new_config)
            end
            {
                scan_consistency: DEFAULT_SCAN_CONSISTENCY,
                adhoc: true
            }.merge(Thread.current['__couchbaseorm_n1ql_config__'] || {})
        end

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