feat: move optional filters to SSSQL and fail fast at runtime - #679
Merged
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 15 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces SSSQL scaffold and refresh commands for authoring optional SQL filters at the SQL level, removes runtime dynamic filter injection from DynamicQueryBuilder (now fail-fast), and adds observed SQL matching capabilities. It refactors optional condition branch detection while maintaining runtime pruning. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CLI
participant Builder as SSSQLFilterBuilder
participant Parser as SelectQueryParser
participant Query as Query Graph
User->>Builder: scaffold(sqlText, {col1: val1, ...})
Builder->>Parser: parse(sqlText)
Parser->>Query: SelectQuery AST
Builder->>Query: resolve filter target columns
Builder->>Query: append (param IS NULL OR col = param)
Query->>Builder: modified query
Builder->>User: return transformed SQL
User->>Builder: refresh(sqlText, {col1: val1, ...})
Builder->>Parser: parse(sqlText)
Parser->>Query: SelectQuery AST
Builder->>Query: collectSupportedOptionalConditionBranches()
Query->>Builder: [existing branches]
Builder->>Query: locate matching branch by param name
alt Branch exists
Builder->>Query: remove from old WHERE, rebase aliases if safe
Builder->>Query: attach to new target query
else Branch missing
Builder->>Query: scaffold new optional branch
end
Query->>Builder: modified query
Builder->>User: return transformed SQL
sequenceDiagram
participant User as User/CLI
participant Report as buildObservedSqlMatchReport()
participant FS as Filesystem
participant Parser as SelectQueryParser
participant Scorer as scoreSummaries()
participant Formatter as formatObservedSqlMatchReport()
User->>Report: observedSql, rootDir
Report->>FS: discoverObservedSqlAssetFiles(rootDir)
FS->>Report: [.sql file paths]
Report->>Parser: parse observed SQL
Parser->>Report: observed summaries
Report->>Parser: parse each candidate .sql file
Parser->>Report: candidate summaries
Report->>Scorer: score(candidate, observed)
Scorer->>Report: match scores
Report->>Report: rank, truncate top-N
Report->>User: ObservedSqlMatchReport
User->>Formatter: format(report, 'text'/'json')
Formatter->>User: human/machine-readable output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
packages/core/src/transformers/SSSQLFilterBuilder.ts (2)
179-182: Unreachable guard condition.After the preceding guards (lines 164-177),
matchesis guaranteed to have exactly one element. Theif (!match) { continue; }check at lines 180-182 can never be true and is dead code.🧹 Remove unreachable guard
const [match] = matches; - if (!match) { - continue; - } + // match is guaranteed to exist after length checks above🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/transformers/SSSQLFilterBuilder.ts` around lines 179 - 182, The guard checking `if (!match) { continue; }` after `const [match] = matches;` in SSSQLFilterBuilder is unreachable because prior checks guarantee `matches` has exactly one element; remove this dead-code check (the `if (!match)` block) and rely on the existing earlier guards that validate `matches`, leaving the code using the `match` variable directly (refer to the `matches` binding and the `const [match] = matches;` statement in SSSQLFilterBuilder).
100-112: DuplicatedcollectTopLevelAndTermslogic.This helper duplicates the same function defined in
PruneOptionalConditionBranches.ts(lines 41-51). Consider importing and reusing the existing implementation to maintain a single source of truth.♻️ Suggested refactor to import shared helper
In
PruneOptionalConditionBranches.ts, export the helper:-const collectTopLevelAndTerms = (expression: ValueComponent): ValueComponent[] => { +export const collectTopLevelAndTerms = (expression: ValueComponent): ValueComponent[] => {Then in
SSSQLFilterBuilder.ts:import { collectSupportedOptionalConditionBranches, + collectTopLevelAndTerms, type SupportedOptionalConditionBranch } from "./PruneOptionalConditionBranches"; -const collectTopLevelAndTerms = (expression: ValueComponent): ValueComponent[] => { - if ( - expression instanceof BinaryExpression && - expression.operator.value.trim().toLowerCase() === "and" - ) { - return [ - ...collectTopLevelAndTerms(expression.left), - ...collectTopLevelAndTerms(expression.right) - ]; - } - - return [expression]; -};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/transformers/SSSQLFilterBuilder.ts` around lines 100 - 112, The function collectTopLevelAndTerms in SSSQLFilterBuilder.ts duplicates logic already implemented in PruneOptionalConditionBranches.ts; replace the duplicate by importing and reusing the single exported helper: export collectTopLevelAndTerms from PruneOptionalConditionBranches.ts (where the current implementation lives) and remove the local implementation in SSSQLFilterBuilder.ts, then update SSSQLFilterBuilder to import collectTopLevelAndTerms and keep existing usage that expects a ValueComponent and handles BinaryExpression with operator.value "and".packages/core/src/transformers/PruneOptionalConditionBranches.ts (1)
199-233: Consider consolidating duplicate traversal logic.
traverseNestedSelectQueries(lines 199-233) andtraverseNestedSelectQueriesForCollection(lines 326-358) share nearly identical structure: cycle-protected object walking withWeakSet, array/object descent, andisSelectQueryNodechecks. The only difference is the callback action performed on discovered queries.A unified traversal helper accepting a visitor callback would reduce duplication and ensure consistent behavior.
♻️ Suggested refactor to unify traversal
+const traverseNestedSelectQueriesWithVisitor = ( + root: SelectQuery, + visitor: (query: SelectQuery) => void +): void => { + const visited = new WeakSet<object>(); + + const walk = (value: unknown): void => { + if (!value || typeof value !== 'object') { + return; + } + + if (visited.has(value as object)) { + return; + } + visited.add(value as object); + + if (value !== root && isSelectQueryNode(value)) { + visitor(value); + return; + } + + if (Array.isArray(value)) { + value.forEach(walk); + return; + } + + for (const child of Object.values(value as Record<string, unknown>)) { + walk(child); + } + }; + + walk(root); +};Then refactor both callers to use this helper.
Also applies to: 326-358
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/transformers/PruneOptionalConditionBranches.ts` around lines 199 - 233, traverseNestedSelectQueries and traverseNestedSelectQueriesForCollection duplicate traversal logic; extract a single helper (e.g., traverseNestedObjectsWithVisitor) that accepts (root, visitor) where visitor is called for each detected select-query node and returns boolean indicating change; implement the same WeakSet cycle protection, array/object descent, and isSelectQueryNode check in the helper, then refactor traverseNestedSelectQueries to call the helper with a visitor that invokes traverseSelectQuery(...) and refactor traverseNestedSelectQueriesForCollection to call the helper with a visitor that invokes traverseSelectQueryForCollection(...), combining returned booleans with logical OR to preserve the original changed semantics.packages/sql-grep-core/src/index.ts (1)
5-11: Redundant explicit re-exports.Line 5 (
export * from './observed/match') already exports all public symbols from that module, includingbuildObservedSqlMatchReport,formatObservedSqlMatchReport, anddiscoverObservedSqlAssetFiles. The explicit named exports at lines 7-11 are redundant.If the intent is to document the key exports for discoverability, consider using a comment instead:
🧹 Remove redundant exports
export * from './observed/match'; export * from './observed/types'; -export { - buildObservedSqlMatchReport, - formatObservedSqlMatchReport, - discoverObservedSqlAssetFiles -} from './observed/match'; +// Key exports from ./observed/match: +// - buildObservedSqlMatchReport +// - formatObservedSqlMatchReport +// - discoverObservedSqlAssetFiles🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sql-grep-core/src/index.ts` around lines 5 - 11, Remove the redundant named re-exports of buildObservedSqlMatchReport, formatObservedSqlMatchReport, and discoverObservedSqlAssetFiles from index.ts since export * from './observed/match' already exposes them; delete the explicit export block that lists those three symbols (referencing the module './observed/match') and, if desired for discoverability, replace it with a short comment listing the key exports.packages/sql-grep-core/src/observed/types.ts (1)
51-57: Consider consolidatingfilesScannedandsqlFilesScannedif they always have the same value.Based on the implementation in
match.ts:193-194, bothfilesScannedandsqlFilesScannedare set tocandidateFiles.length. If these will always be equal, consider removing one to simplify the API. If they're intended to diverge in the future (e.g., scanning non-SQL files), adding a brief comment documenting the distinction would be helpful.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sql-grep-core/src/observed/types.ts` around lines 51 - 57, The summary type currently exposes both filesScanned and sqlFilesScanned which are set to the same value in the implementation (see match.ts where both are assigned candidateFiles.length); either remove one of the fields (e.g., drop filesScanned and keep sqlFilesScanned) and update all usages/types accordingly, or if they are meant to diverge later, add a short inline comment on the summary type near filesScanned/sqlFilesScanned explaining the intended distinction (e.g., filesScanned = all files processed, sqlFilesScanned = only SQL files) and keep both fields; update any consumers of the Summary type (and tests) to match the chosen change.packages/ztd-cli/src/commands/query.ts (1)
541-569: Consider adding user feedback when no existing branches are found during refresh.When
collectSupportedOptionalConditionBranches(parsed)returns an empty array (line 544-545), thefiltersobject will be empty, andrefreshwill effectively be a no-op. Consider logging a warning or returning early with a message when there are no branches to refresh.💡 Optional enhancement
function runQuerySssqlRefreshCommand(sqlFile: string, options: QuerySssqlRefreshOptions): void { const sql = readFileSync(sqlFile, 'utf8'); const parsed = SelectQueryParser.parse(sql); const existingBranches = collectSupportedOptionalConditionBranches(parsed); + if (existingBranches.length === 0) { + const format = normalizeFormat(normalizeStringOption(options.format) ?? getAgentOutputFormat()); + if (format === 'json') { + writeCommandEnvelope('query sssql refresh', { + file: sqlFile, + output_file: null, + written: false, + sql: null, + warning: 'No existing optional filter branches found to refresh.' + }); + return; + } + console.warn('No existing optional filter branches found to refresh.'); + return; + } const filters = Object.fromEntries(existingBranches.map((branch) => [branch.parameterName, null]));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ztd-cli/src/commands/query.ts` around lines 541 - 569, In runQuerySssqlRefreshCommand, detect when collectSupportedOptionalConditionBranches(parsed) returns an empty array and short-circuit: if existingBranches.length === 0, emit a clear user-facing message (use writeCommandEnvelope when format === 'json' to keep CLI JSON output consistent, otherwise write a warning to stderr or stdout) and return early instead of calling SSSQLFilterBuilder.refresh with an empty filters object; update references around existingBranches, filters, and the format checks so the CLI informs the user there were no branches to refresh.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sql-grep-core/src/observed/match.ts`:
- Around line 664-675: The ranking currently ignores set-operation mismatches;
include the result of compareTokenSets for setOperationTokens (the variable
likely named setOperation from compareTokenSets(observedSetOperationTokens,
candidateSetOperationTokens)) into the aggregate score calculation or apply an
explicit penalty when setOperation.score indicates mismatch. Update the score
formula that builds the final score (the block computing projection, source,
where, order, paging and the final score variable) to incorporate
setOperation.score with an appropriate weight or subtract a fixed penalty when
setOperation.score < 1; apply the same change to the analogous score computation
at the other location referenced (around lines 690-692) so set-operation shape
mismatches reduce the rank.
- Around line 598-600: The current FunctionCall branch in operand serialization
(the block that checks `candidate instanceof FunctionCall` and returns
`fn:${normalizeFunctionName(candidate)}`) discards the function arguments
causing different calls like `lower(status)` and `lower(email)` to collide;
update this branch to include a stable representation of the function arguments
(e.g., append a parenthesized, normalized serialization of `candidate.args` or a
single serialized operand for the first arg) when building the operand key so
the returned string encodes both `normalizeFunctionName(candidate)` and its
argument(s). Locate the FunctionCall handling code and use existing helpers
(like the operand rendering/normalization code or `normalizeFunctionName`) to
produce `fn:<name>(<arg-repr>)` or equivalent to preserve argument identity in
predicates.
- Around line 562-569: The serializer currently preserves left/right order for
commutative boolean operators in the BinaryExpression branch (operator === 'and'
|| operator === 'or'), causing logically equivalent predicates to serialize
differently; change the logic in the BinaryExpression handling (the branch
around BinaryExpression, normalizePredicateSignature, and
normalizePredicateOperand) to canonicalize commutative boolean trees by:
recursively flattening nested BinaryExpression nodes that share the same
operator into a single operand list, computing the normalized signature for each
operand via normalizePredicateSignature, sorting those operand signatures
deterministically, and then joining them with '|' inside the operator(...)
string so (a OR b) and (b OR a) produce the same token. Ensure the flattening
only applies when operator === 'and' or 'or' and leaves non-commutative
operators unchanged.
---
Nitpick comments:
In `@packages/core/src/transformers/PruneOptionalConditionBranches.ts`:
- Around line 199-233: traverseNestedSelectQueries and
traverseNestedSelectQueriesForCollection duplicate traversal logic; extract a
single helper (e.g., traverseNestedObjectsWithVisitor) that accepts (root,
visitor) where visitor is called for each detected select-query node and returns
boolean indicating change; implement the same WeakSet cycle protection,
array/object descent, and isSelectQueryNode check in the helper, then refactor
traverseNestedSelectQueries to call the helper with a visitor that invokes
traverseSelectQuery(...) and refactor traverseNestedSelectQueriesForCollection
to call the helper with a visitor that invokes
traverseSelectQueryForCollection(...), combining returned booleans with logical
OR to preserve the original changed semantics.
In `@packages/core/src/transformers/SSSQLFilterBuilder.ts`:
- Around line 179-182: The guard checking `if (!match) { continue; }` after
`const [match] = matches;` in SSSQLFilterBuilder is unreachable because prior
checks guarantee `matches` has exactly one element; remove this dead-code check
(the `if (!match)` block) and rely on the existing earlier guards that validate
`matches`, leaving the code using the `match` variable directly (refer to the
`matches` binding and the `const [match] = matches;` statement in
SSSQLFilterBuilder).
- Around line 100-112: The function collectTopLevelAndTerms in
SSSQLFilterBuilder.ts duplicates logic already implemented in
PruneOptionalConditionBranches.ts; replace the duplicate by importing and
reusing the single exported helper: export collectTopLevelAndTerms from
PruneOptionalConditionBranches.ts (where the current implementation lives) and
remove the local implementation in SSSQLFilterBuilder.ts, then update
SSSQLFilterBuilder to import collectTopLevelAndTerms and keep existing usage
that expects a ValueComponent and handles BinaryExpression with operator.value
"and".
In `@packages/sql-grep-core/src/index.ts`:
- Around line 5-11: Remove the redundant named re-exports of
buildObservedSqlMatchReport, formatObservedSqlMatchReport, and
discoverObservedSqlAssetFiles from index.ts since export * from
'./observed/match' already exposes them; delete the explicit export block that
lists those three symbols (referencing the module './observed/match') and, if
desired for discoverability, replace it with a short comment listing the key
exports.
In `@packages/sql-grep-core/src/observed/types.ts`:
- Around line 51-57: The summary type currently exposes both filesScanned and
sqlFilesScanned which are set to the same value in the implementation (see
match.ts where both are assigned candidateFiles.length); either remove one of
the fields (e.g., drop filesScanned and keep sqlFilesScanned) and update all
usages/types accordingly, or if they are meant to diverge later, add a short
inline comment on the summary type near filesScanned/sqlFilesScanned explaining
the intended distinction (e.g., filesScanned = all files processed,
sqlFilesScanned = only SQL files) and keep both fields; update any consumers of
the Summary type (and tests) to match the chosen change.
In `@packages/ztd-cli/src/commands/query.ts`:
- Around line 541-569: In runQuerySssqlRefreshCommand, detect when
collectSupportedOptionalConditionBranches(parsed) returns an empty array and
short-circuit: if existingBranches.length === 0, emit a clear user-facing
message (use writeCommandEnvelope when format === 'json' to keep CLI JSON output
consistent, otherwise write a warning to stderr or stdout) and return early
instead of calling SSSQLFilterBuilder.refresh with an empty filters object;
update references around existingBranches, filters, and the format checks so the
CLI informs the user there were no branches to refresh.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 42de9246-57cb-4904-b812-aaee5e489dc5
📒 Files selected for processing (18)
.changeset/soft-tomatoes-explain.mdREADME.mddocs/guide/sql-tool-happy-paths.mdpackages/core/src/index.tspackages/core/src/transformers/DynamicQueryBuilder.tspackages/core/src/transformers/PruneOptionalConditionBranches.tspackages/core/src/transformers/SSSQLFilterBuilder.tspackages/core/tests/transformers/DynamicFilterRoutingDogfooding.test.tspackages/core/tests/transformers/DynamicQueryBuilder.test.tspackages/core/tests/transformers/PruneOptionalConditionBranches.test.tspackages/core/tests/transformers/SSSQLFilterBuilder.test.tspackages/sql-grep-core/src/index.tspackages/sql-grep-core/src/observed/match.tspackages/sql-grep-core/src/observed/types.tspackages/sql-grep-core/tests/observedSqlMatch.test.tspackages/ztd-cli/README.mdpackages/ztd-cli/src/commands/query.tspackages/ztd-cli/tests/cliCommands.test.ts
Comment on lines
+562
to
+569
| if (candidate instanceof BinaryExpression) { | ||
| const operator = normalizeIdentifier(candidate.operator.value); | ||
| if (isComparisonOperator(operator)) { | ||
| return `${operator}(${normalizePredicateOperand(candidate.left, aliasMap)}|${normalizePredicateOperand(candidate.right, aliasMap)})`; | ||
| } | ||
| if (operator === 'and' || operator === 'or') { | ||
| return `${operator}(${normalizePredicateSignature(candidate.left, aliasMap)}|${normalizePredicateSignature(candidate.right, aliasMap)})`; | ||
| } |
There was a problem hiding this comment.
Canonicalize nested AND/OR branches.
This serializer keeps left/right order for boolean operators, so (a OR b) and (b OR a) become different where tokens. That will lower match scores for logically equivalent predicates that were only written in a different order.
Suggested fix
if (candidate instanceof BinaryExpression) {
const operator = normalizeIdentifier(candidate.operator.value);
if (isComparisonOperator(operator)) {
return `${operator}(${normalizePredicateOperand(candidate.left, aliasMap)}|${normalizePredicateOperand(candidate.right, aliasMap)})`;
}
if (operator === 'and' || operator === 'or') {
- return `${operator}(${normalizePredicateSignature(candidate.left, aliasMap)}|${normalizePredicateSignature(candidate.right, aliasMap)})`;
+ const operands = [
+ normalizePredicateSignature(candidate.left, aliasMap),
+ normalizePredicateSignature(candidate.right, aliasMap)
+ ].sort();
+ return `${operator}(${operands.join('|')})`;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (candidate instanceof BinaryExpression) { | |
| const operator = normalizeIdentifier(candidate.operator.value); | |
| if (isComparisonOperator(operator)) { | |
| return `${operator}(${normalizePredicateOperand(candidate.left, aliasMap)}|${normalizePredicateOperand(candidate.right, aliasMap)})`; | |
| } | |
| if (operator === 'and' || operator === 'or') { | |
| return `${operator}(${normalizePredicateSignature(candidate.left, aliasMap)}|${normalizePredicateSignature(candidate.right, aliasMap)})`; | |
| } | |
| if (candidate instanceof BinaryExpression) { | |
| const operator = normalizeIdentifier(candidate.operator.value); | |
| if (isComparisonOperator(operator)) { | |
| return `${operator}(${normalizePredicateOperand(candidate.left, aliasMap)}|${normalizePredicateOperand(candidate.right, aliasMap)})`; | |
| } | |
| if (operator === 'and' || operator === 'or') { | |
| const operands = [ | |
| normalizePredicateSignature(candidate.left, aliasMap), | |
| normalizePredicateSignature(candidate.right, aliasMap) | |
| ].sort(); | |
| return `${operator}(${operands.join('|')})`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sql-grep-core/src/observed/match.ts` around lines 562 - 569, The
serializer currently preserves left/right order for commutative boolean
operators in the BinaryExpression branch (operator === 'and' || operator ===
'or'), causing logically equivalent predicates to serialize differently; change
the logic in the BinaryExpression handling (the branch around BinaryExpression,
normalizePredicateSignature, and normalizePredicateOperand) to canonicalize
commutative boolean trees by: recursively flattening nested BinaryExpression
nodes that share the same operator into a single operand list, computing the
normalized signature for each operand via normalizePredicateSignature, sorting
those operand signatures deterministically, and then joining them with '|'
inside the operator(...) string so (a OR b) and (b OR a) produce the same token.
Ensure the flattening only applies when operator === 'and' or 'or' and leaves
non-commutative operators unchanged.
Comment on lines
+598
to
+600
| if (candidate instanceof FunctionCall) { | ||
| return `fn:${normalizeFunctionName(candidate)}`; | ||
| } |
There was a problem hiding this comment.
Preserve function arguments in predicate operands.
Right now lower(status) and lower(email) both normalize to fn:lower, so different filters can compare as identical whenever they share the same wrapper function. That makes where scoring too lossy for common patterns like lower(...) LIKE lower(:param).
Suggested fix
if (candidate instanceof FunctionCall) {
- return `fn:${normalizeFunctionName(candidate)}`;
+ const args: string[] = [];
+ if (candidate.argument) {
+ args.push(normalizePredicateOperand(candidate.argument, aliasMap));
+ }
+ if (candidate.filterCondition) {
+ args.push(`filter:${normalizePredicateSignature(candidate.filterCondition, aliasMap)}`);
+ }
+ return `fn:${normalizeFunctionName(candidate)}(${args.join('|')})`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/sql-grep-core/src/observed/match.ts` around lines 598 - 600, The
current FunctionCall branch in operand serialization (the block that checks
`candidate instanceof FunctionCall` and returns
`fn:${normalizeFunctionName(candidate)}`) discards the function arguments
causing different calls like `lower(status)` and `lower(email)` to collide;
update this branch to include a stable representation of the function arguments
(e.g., append a parenthesized, normalized serialization of `candidate.args` or a
single serialized operand for the first arg) when building the operand key so
the returned string encodes both `normalizeFunctionName(candidate)` and its
argument(s). Locate the FunctionCall handling code and use existing helpers
(like the operand rendering/normalization code or `normalizeFunctionName`) to
produce `fn:<name>(<arg-repr>)` or equivalent to preserve argument identity in
predicates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR moves optional filter authoring to SSSQL scaffold/refresh, makes legacy runtime filter injection fail fast, and keeps runtime pruning plus sort/paging responsibilities intact.\n\nCloses #632
Summary by CodeRabbit
Release Notes
New Features
ztd query sssql scaffoldandztd query sssql refreshcommands for SQL-first optional filter authoringztd query match-observedcommand to discover similar SQL queries in your codebaseBreaking Changes
sssqlcommands insteadDocumentation