fix(crud): append default opts map to varargs upsert for CRUD 1.5.0#101
Draft
dkasimovskiy wants to merge 1 commit into
Draft
fix(crud): append default opts map to varargs upsert for CRUD 1.5.0#101dkasimovskiy wants to merge 1 commit into
dkasimovskiy wants to merge 1 commit into
Conversation
The varargs \`upsert(Options, Object...)\` and \`upsertObject(Options, Object...)\` overloads forwarded their \`arguments\` straight to \`iprotoCall\`, producing a 3-arg wire call \`crud.upsert(space, tuple, operations)\` with no opts table. CRUD rock 1.5.0+ (bundled with Tarantool 3.5.0) rejects that 3-arg form at \`crud/compare/conditions.lua:93\` with \`ParseConditionError: Each condition should be table, got "string" (condition 1)\`. The 4-arg form \`(space, tuple, operations, opts)\` works because that overload delegates through \`toTupleOperationsOptsArgs\`. Fix: append \`DEFAULT_UPDATE_OPTIONS.getOptions()\` to the args when the caller has not already supplied an opts map (last varargs element not a \`Map\`). The guard preserves the existing 4-arg call from \`testUpsertParameters\`, which deliberately passes its own options map. Fixes \`TarantoolCrudClientTest#testUpsert\` on Tarantool 3.5.0. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
dkasimovskiy
force-pushed
the
fix/crud-upsert-parse-error-3.5.0
branch
from
56c93ab to
efcf1ca
Compare
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.
The varargs
upsert(Options, Object...)andupsertObject(Options, Object...)overloads forwarded theirargumentsstraight toiprotoCall, producing a 3-arg wire callcrud.upsert(space, tuple, operations)with no opts table.CRUD rock 1.5.0 (bundled with Tarantool 3.5.0) interprets that 3-arg form differently:
crud/compare/conditions.lua:93rejects the operations list withParseConditionError: Each condition should be table, got "string" (condition 1). The 4-arg form(space, tuple, operations, opts)works because that overload delegates throughtoTupleOperationsOptsArgs.Fix: append
DEFAULT_UPDATE_OPTIONS.getOptions()to the argument list when the caller has not already supplied an opts map (detected by checking whether the last varargs element is aMap). Theinstanceof Mapguard preserves the existing 4-arg call fromtestUpsertParameters, which deliberately passes its own options map.Fixes
TarantoolCrudClientTest#testUpserton Tarantool 3.5.0.I haven't forgotten about:
Related issues: