-
Notifications
You must be signed in to change notification settings - Fork 10
Add storage.randomAccessFields config to disable typed structures (v5.0) #1169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,179 @@ | ||
| require('../testUtils'); | ||
| const assert = require('assert'); | ||
| const { RecordEncoder, IndexRecordEncoder } = require('#src/resources/RecordEncoder'); | ||
| const env = require('#js/utility/environment/environmentManager'); | ||
| const terms = require('#src/utility/hdbTerms'); | ||
|
|
||
| // In-memory shared structures (mirrors how a DBI shares the structures array under | ||
| // Symbol.for('structures')) so struct/record structures cross between encoder instances. We keep the | ||
| // live array reference rather than round-tripping through encode/decode, since msgpackr attaches | ||
| // bookkeeping to the structures array that a re-decode would strip. | ||
| function sharedStore() { | ||
| let structures = []; | ||
| return { | ||
| save(s) { | ||
| structures = s; | ||
| return true; | ||
| }, | ||
| get() { | ||
| return structures; | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| function makeEncoder(store, extra) { | ||
| return new RecordEncoder({ | ||
| // randomAccessStructure stays on regardless of the opt-out — reads must keep decoding typed | ||
| // structs so existing data is still readable; only writes change. | ||
| randomAccessStructure: true, | ||
| getStructures: store.get, | ||
| saveStructures: store.save, | ||
| ...extra, | ||
| }); | ||
| } | ||
|
|
||
| const record = { name: 'price', type: 'Float', indexed: true }; | ||
|
|
||
| describe('RecordEncoder random-access fields opt-out (readOnlyStructures)', () => { | ||
| it('writes typed random-access structs by default (random-access fields on)', () => { | ||
| const store = sharedStore(); | ||
| const enc = makeEncoder(store); | ||
| const bytes = enc.encode(record); | ||
| assert.ok(bytes[0] >= 0x20 && bytes[0] < 0x40, `expected typed-struct header byte, got 0x${bytes[0].toString(16)}`); | ||
| }); | ||
|
|
||
| it('writes classic shared structures when readOnlyStructures is set (opt-out)', () => { | ||
| const store = sharedStore(); | ||
| const enc = makeEncoder(store, { readOnlyStructures: true }); | ||
| const bytes = enc.encode(record); | ||
| assert.ok( | ||
| bytes[0] >= 0x40 && bytes[0] < 0x80, | ||
| `expected classic shared-structure byte, got 0x${bytes[0].toString(16)}` | ||
| ); | ||
| }); | ||
|
|
||
| it('opt-out encoder still reads typed-struct data written before opting out', () => { | ||
| // Existing data was written as typed structs; after the user opts out the same store must keep | ||
| // decoding it (randomAccessStructure stays on), only new writes switch to classic. | ||
| const store = sharedStore(); | ||
| const typedWriter = makeEncoder(store); | ||
| const typedBytes = typedWriter.encode(record); | ||
| assert.ok(typedBytes[0] >= 0x20 && typedBytes[0] < 0x40, 'precondition: typed-struct bytes'); | ||
|
|
||
| const optedOutReader = makeEncoder(store, { readOnlyStructures: true }); | ||
| const decoded = optedOutReader.decode(Buffer.from(typedBytes)); | ||
| assert.ok(decoded, 'typed-struct data should still decode (not swallowed to null)'); | ||
| assert.strictEqual(decoded.name, record.name); | ||
| assert.strictEqual(decoded.type, record.type); | ||
| assert.strictEqual(decoded.indexed, record.indexed); | ||
| }); | ||
|
|
||
| it('classic records and typed structs round-trip together on the same store', () => { | ||
| const store = sharedStore(); | ||
| const typedWriter = makeEncoder(store); | ||
| const classicWriter = makeEncoder(store, { readOnlyStructures: true }); | ||
| const reader = makeEncoder(store); | ||
|
|
||
| const a = { id: 1, kind: 'typed' }; | ||
| const b = { id: 2, kind: 'classic', extra: true }; | ||
| const typedBytes = typedWriter.encode(a); | ||
| const classicBytes = classicWriter.encode(b); | ||
| assert.ok(typedBytes[0] >= 0x20 && typedBytes[0] < 0x40, 'precondition: typed bytes'); | ||
| assert.ok(classicBytes[0] >= 0x40 && classicBytes[0] < 0x80, 'precondition: classic bytes'); | ||
|
|
||
| // Typed structs decode with the RecordObject prototype, classic records as plain objects; spread | ||
| // to compare field values regardless of prototype. | ||
| assert.deepStrictEqual({ ...reader.decode(Buffer.from(typedBytes)) }, a); | ||
| assert.deepStrictEqual({ ...reader.decode(Buffer.from(classicBytes)) }, b); | ||
| }); | ||
|
|
||
| it('IndexRecordEncoder keeps writing typed structs even with readOnlyStructures requested', () => { | ||
| // Object-store indexes (HNSW) must stay in struct mode regardless of the table opt-out. | ||
| const store = sharedStore(); | ||
| const enc = new IndexRecordEncoder({ | ||
| randomAccessStructure: true, | ||
| readOnlyStructures: true, | ||
| getStructures: store.get, | ||
| saveStructures: store.save, | ||
| }); | ||
| const bytes = enc.encode(record); | ||
| assert.ok( | ||
| bytes[0] >= 0x20 && bytes[0] < 0x40, | ||
| `index encoder should ignore readOnlyStructures and write typed structs, got 0x${bytes[0].toString(16)}` | ||
| ); | ||
| }); | ||
|
|
||
| it('decodes a classic record whose structure-id byte is 66 (0x42) when noMetadata is set', () => { | ||
| // Regression: 66 (0x42) is classic shared-structure record-id #2 and also a rocksdb local-timestamp | ||
| // marker. On a rocksdb store the prefix heuristic strips 8 bytes and corrupts the record, decoding | ||
| // to null (the MQTT "publish non-JSON" failure). The audit store passes { noMetadata: true } to skip | ||
| // the heuristic for values that have no on-disk timestamp prefix. | ||
| const store = sharedStore(); | ||
| const writer = makeEncoder(store, { readOnlyStructures: true }); | ||
| // The 3rd distinct classic structure starts with byte 0x42; encode three shapes to reach it. | ||
| writer.encode({ a: 1 }); | ||
| writer.encode({ b: 2, c: 3 }); | ||
| const target = { d: 4, e: 5, f: 6 }; | ||
| const bytes = Buffer.from(writer.encode(target)); | ||
| assert.strictEqual(bytes[0], 66, 'precondition: target record begins with structure-id byte 66 (0x42)'); | ||
|
|
||
| const reader = makeEncoder(store); | ||
| // Warm up the structure cache with a normal (non-rocksdb) decode so getStructures isn't needed | ||
| // once we flip isRocksDB below (the rocksdb getStructures path needs a rootStore we don't mock). | ||
| assert.deepStrictEqual(reader.decode(bytes), target, 'baseline classic decode works'); | ||
|
|
||
| // Simulate the rocksdb decode path, where byte 66 collides with the local-timestamp marker. | ||
| reader.isRocksDB = true; | ||
| assert.deepStrictEqual( | ||
| reader.decode(bytes, { noMetadata: true }), | ||
| target, | ||
| 'with noMetadata the classic record still decodes correctly' | ||
| ); | ||
| // Without noMetadata the rocksdb timestamp heuristic misreads byte 66 and corrupts the decode. | ||
| assert.notDeepStrictEqual( | ||
| reader.decode(bytes), | ||
| target, | ||
| 'without noMetadata the 0x42 collision corrupts the decode (demonstrating why the flag is needed)' | ||
| ); | ||
| }); | ||
|
|
||
| describe('storage.randomAccessFields config drives the default', () => { | ||
| let previous; | ||
| beforeEach(() => { | ||
| previous = env.get(terms.CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS); | ||
| }); | ||
| afterEach(() => { | ||
| env.setProperty(terms.CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS, previous); | ||
| }); | ||
|
|
||
| it('writes classic structures when storage.randomAccessFields is false (no explicit option)', () => { | ||
| env.setProperty(terms.CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS, false); | ||
| const enc = makeEncoder(sharedStore()); // no explicit readOnlyStructures | ||
| const bytes = enc.encode(record); | ||
| assert.ok( | ||
| bytes[0] >= 0x40 && bytes[0] < 0x80, | ||
| `config off should write classic structures, got 0x${bytes[0].toString(16)}` | ||
| ); | ||
| }); | ||
|
|
||
| it('writes typed structs when storage.randomAccessFields is true', () => { | ||
| env.setProperty(terms.CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS, true); | ||
| const enc = makeEncoder(sharedStore()); | ||
| const bytes = enc.encode(record); | ||
| assert.ok( | ||
| bytes[0] >= 0x20 && bytes[0] < 0x40, | ||
| `config on should write typed structs, got 0x${bytes[0].toString(16)}` | ||
| ); | ||
| }); | ||
|
|
||
| it('an explicit readOnlyStructures option overrides the config', () => { | ||
| env.setProperty(terms.CONFIG_PARAMS.STORAGE_RANDOMACCESSFIELDS, false); | ||
| const enc = makeEncoder(sharedStore(), { readOnlyStructures: false }); | ||
| const bytes = enc.encode(record); | ||
| assert.ok( | ||
| bytes[0] >= 0x20 && bytes[0] < 0x40, | ||
| `explicit readOnlyStructures:false should keep typed structs despite config, got 0x${bytes[0].toString(16)}` | ||
| ); | ||
| }); | ||
| }); | ||
| }); | ||
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small repo-standard nit: new tests should import Node's built-in assert module with the
node:prefix. Eithernode:assertornode:assert/strictworks; the important part is avoiding the bareassertpackage name.\n\nsuggestion\nconst assert = require('node:assert/strict');\n\n\nCodex review