From 23ef2105deef63b19ba753803d1f050c2e3e654d Mon Sep 17 00:00:00 2001 From: muraliQlogic Date: Mon, 5 Nov 2018 00:43:39 +0530 Subject: [PATCH 1/6] Fix for Issue#343 --- src/chunktransformer.js | 9 --------- system-test/read-rows-acceptance-test.json | 13 ------------- test/chunktransformer.js | 9 --------- 3 files changed, 31 deletions(-) diff --git a/src/chunktransformer.js b/src/chunktransformer.js index d899c047d..16c24bfb0 100644 --- a/src/chunktransformer.js +++ b/src/chunktransformer.js @@ -248,15 +248,6 @@ class ChunkTransformer extends Transform { return; } } - if (chunk.familyName && !chunk.qualifier) { - this.destroy( - new TransformError({ - message: 'A qualifier must be specified', - chunk, - }) - ); - return; - } this.validateResetRow(chunk); this.validateValueSizeAndCommitRow(chunk); } diff --git a/system-test/read-rows-acceptance-test.json b/system-test/read-rows-acceptance-test.json index e0cd829ad..ce02f5866 100644 --- a/system-test/read-rows-acceptance-test.json +++ b/system-test/read-rows-acceptance-test.json @@ -38,19 +38,6 @@ "value": "", "ts": 0 }] - }, { - "chunks": ["row_key: \"RK\"\nfamily_name: <\n value: \"A\"\n>\nqualifier: <\n value: \"C\"\n>\ntimestamp_micros: 99\nvalue: \"value-VAL_1\"\ncommit_row: false\n", "family_name: <\n value: \"B\"\n>\ntimestamp_micros: 98\nvalue: \"value-VAL_2\"\ncommit_row: true\n"], - "name": "invalid - new col family must specify qualifier", - "chunks_base64": ["CgJSSxIDCgFBGgMKAUMgYzILdmFsdWUtVkFMXzFIAA==", "EgMKAUIgYjILdmFsdWUtVkFMXzJIAQ=="], - "results": [{ - "fm": "", - "rk": "", - "qual": "", - "label": "", - "error": true, - "value": "", - "ts": 0 - }] }, { "chunks": ["row_key: \"RK\"\nfamily_name: <\n value: \"A\"\n>\nqualifier: <\n value: \"C\"\n>\ntimestamp_micros: 100\nvalue: \"value-VAL\"\ncommit_row: false\n", "commit_row: true\n"], "name": "bare commit implies ts=0", diff --git a/test/chunktransformer.js b/test/chunktransformer.js index 7ff9a5bfe..93eb55f8a 100644 --- a/test/chunktransformer.js +++ b/test/chunktransformer.js @@ -369,15 +369,6 @@ describe('Bigtable/ChunkTransformer', function() { commitRow: true, }); }); - it('should destroy when familyName without qualifier ', function(done) { - chunkTransformer.on('error', function() { - assert(destroySpy.called); - done(); - }); - processRowInProgressSpy.call(chunkTransformer, { - familyName: 'family', - }); - }); it('should reset on resetRow ', function() { const chunk = {resetRow: true}; chunkTransformer.processRowInProgress(chunk); From 81cb106541e195ec5b8f0a3b7b3295e23c8a6974 Mon Sep 17 00:00:00 2001 From: muraliQlogic Date: Tue, 6 Nov 2018 23:59:57 +0530 Subject: [PATCH 2/6] Reinstate changes and applied modified check to allow empty qualifier --- src/chunktransformer.js | 12 +++++ system-test/read-rows-acceptance-test.json | 13 +++++ test/chunktransformer.js | 56 ++++++++++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/src/chunktransformer.js b/src/chunktransformer.js index 16c24bfb0..a2125ef4e 100644 --- a/src/chunktransformer.js +++ b/src/chunktransformer.js @@ -248,6 +248,18 @@ class ChunkTransformer extends Transform { return; } } + if ( + chunk.familyName && + (null === chunk.qualifier || undefined === chunk.qualifier) + ) { + this.destroy( + new TransformError({ + message: 'A qualifier must be specified', + chunk, + }) + ); + return; + } this.validateResetRow(chunk); this.validateValueSizeAndCommitRow(chunk); } diff --git a/system-test/read-rows-acceptance-test.json b/system-test/read-rows-acceptance-test.json index ce02f5866..308cbc815 100644 --- a/system-test/read-rows-acceptance-test.json +++ b/system-test/read-rows-acceptance-test.json @@ -38,6 +38,19 @@ "value": "", "ts": 0 }] + }, { + "chunks": ["row_key: \"RK\"\nfamily_name: <\n value: \"A\"\n>\nqualifier: <\n value: \"C\"\n>\ntimestamp_micros: 99\nvalue: \"value-VAL_1\"\ncommit_row: false\n", "family_name: <\n value: \"B\"\n>\ntimestamp_micros: 98\nvalue: \"value-VAL_2\"\ncommit_row: true\n"], + "name": "invalid - new col family must specify qualifier", + "chunks_base64": ["CgJSSxIDCgFBGgMKAUMgYzILdmFsdWUtVkFMXzFIAA==", "EgMKAUIgYjILdmFsdWUtVkFMXzJIAQ=="], + "results": [{ + "fm": "", + "rk": "", + "qual": "", + "label": "", + "error": true, + "value": "", + "ts": 0 + }] }, { "chunks": ["row_key: \"RK\"\nfamily_name: <\n value: \"A\"\n>\nqualifier: <\n value: \"C\"\n>\ntimestamp_micros: 100\nvalue: \"value-VAL\"\ncommit_row: false\n", "commit_row: true\n"], "name": "bare commit implies ts=0", diff --git a/test/chunktransformer.js b/test/chunktransformer.js index 93eb55f8a..898608daf 100644 --- a/test/chunktransformer.js +++ b/test/chunktransformer.js @@ -369,6 +369,15 @@ describe('Bigtable/ChunkTransformer', function() { commitRow: true, }); }); + it('should destroy when familyName without qualifier ', function(done) { + chunkTransformer.on('error', function() { + assert(destroySpy.called); + done(); + }); + processRowInProgressSpy.call(chunkTransformer, { + familyName: 'family', + }); + }); it('should reset on resetRow ', function() { const chunk = {resetRow: true}; chunkTransformer.processRowInProgress(chunk); @@ -459,6 +468,53 @@ describe('Bigtable/ChunkTransformer', function() { 'state mismatch' ); }); + it('chunk with familyName and empty qualifier should produce row', function() { + chunkTransformer.qualifiers = []; + chunkTransformer.family = { + qualifier: chunkTransformer.qualifiers, + }; + chunkTransformer.row = { + key: 'key', + data: { + family: chunkTransformer.family, + }, + }; + const chunk = { + commitRow: true, + familyName: {value: 'family2'}, + qualifier: '', + value: 'value', + timestampMicros: 0, + labels: [], + valueSize: 0, + }; + chunkTransformer.processRowInProgress(chunk); + assert(commitSpy.called, 'did not call commit'); + assert(resetSpy.called, 'did not call reset'); + assert.strictEqual(rows.length, 1, 'wrong call to push'); + const expectedRow = { + key: 'key', + data: { + family: { + qualifier: [ + { + value: 'value', + timestamp: 0, + labels: [], + }, + ], + }, + family2: {}, + }, + }; + const row = rows[0]; + assert.deepStrictEqual(row, expectedRow, 'row mismatch'); + assert.strictEqual( + chunkTransformer.state, + RowStateEnum.NEW_ROW, + 'state mismatch' + ); + }); it('chunk with new family and commitRow should produce row', function() { chunkTransformer.qualifiers = []; chunkTransformer.family = { From b4d0284a3421acf8558c4f5b99c6457686b8d22c Mon Sep 17 00:00:00 2001 From: Solomon Duskis Date: Tue, 6 Nov 2018 14:46:06 -0800 Subject: [PATCH 3/6] Update read-rows-acceptance-test.json --- system-test/read-rows-acceptance-test.json | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/system-test/read-rows-acceptance-test.json b/system-test/read-rows-acceptance-test.json index 308cbc815..011b97385 100644 --- a/system-test/read-rows-acceptance-test.json +++ b/system-test/read-rows-acceptance-test.json @@ -38,19 +38,19 @@ "value": "", "ts": 0 }] - }, { + }, { "chunks": ["row_key: \"RK\"\nfamily_name: <\n value: \"A\"\n>\nqualifier: <\n value: \"C\"\n>\ntimestamp_micros: 99\nvalue: \"value-VAL_1\"\ncommit_row: false\n", "family_name: <\n value: \"B\"\n>\ntimestamp_micros: 98\nvalue: \"value-VAL_2\"\ncommit_row: true\n"], - "name": "invalid - new col family must specify qualifier", - "chunks_base64": ["CgJSSxIDCgFBGgMKAUMgYzILdmFsdWUtVkFMXzFIAA==", "EgMKAUIgYjILdmFsdWUtVkFMXzJIAQ=="], - "results": [{ - "fm": "", - "rk": "", - "qual": "", - "label": "", - "error": true, - "value": "", - "ts": 0 - }] + "name": "invalid - new col family must specify qualifier", + "chunks_base64": ["CgJSSxIDCgFBGgMKAUMgYzILdmFsdWUtVkFMXzFIAA==", "EgMKAUIgYjILdmFsdWUtVkFMXzJIAQ=="], + "results": [{ + "fm": "", + "rk": "", + "qual": "", + "label": "", + "error": true, + "value": "", + "ts": 0 + }] }, { "chunks": ["row_key: \"RK\"\nfamily_name: <\n value: \"A\"\n>\nqualifier: <\n value: \"C\"\n>\ntimestamp_micros: 100\nvalue: \"value-VAL\"\ncommit_row: false\n", "commit_row: true\n"], "name": "bare commit implies ts=0", @@ -872,4 +872,4 @@ "ts": 0 }] }] -} \ No newline at end of file +} From 108714249b853bf4afd7389451f23bec27f35acb Mon Sep 17 00:00:00 2001 From: Solomon Duskis Date: Tue, 6 Nov 2018 14:46:36 -0800 Subject: [PATCH 4/6] Update read-rows-acceptance-test.json --- system-test/read-rows-acceptance-test.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system-test/read-rows-acceptance-test.json b/system-test/read-rows-acceptance-test.json index 011b97385..62c78a368 100644 --- a/system-test/read-rows-acceptance-test.json +++ b/system-test/read-rows-acceptance-test.json @@ -39,7 +39,7 @@ "ts": 0 }] }, { - "chunks": ["row_key: \"RK\"\nfamily_name: <\n value: \"A\"\n>\nqualifier: <\n value: \"C\"\n>\ntimestamp_micros: 99\nvalue: \"value-VAL_1\"\ncommit_row: false\n", "family_name: <\n value: \"B\"\n>\ntimestamp_micros: 98\nvalue: \"value-VAL_2\"\ncommit_row: true\n"], + "chunks": ["row_key: \"RK\"\nfamily_name: <\n value: \"A\"\n>\nqualifier: <\n value: \"C\"\n>\ntimestamp_micros: 99\nvalue: \"value-VAL_1\"\ncommit_row: false\n", "family_name: <\n value: \"B\"\n>\ntimestamp_micros: 98\nvalue: \"value-VAL_2\"\ncommit_row: true\n"], "name": "invalid - new col family must specify qualifier", "chunks_base64": ["CgJSSxIDCgFBGgMKAUMgYzILdmFsdWUtVkFMXzFIAA==", "EgMKAUIgYjILdmFsdWUtVkFMXzJIAQ=="], "results": [{ From 615072762ecbcc24d2dce3ff2962b5d240615896 Mon Sep 17 00:00:00 2001 From: Solomon Duskis Date: Thu, 8 Nov 2018 09:21:21 -0500 Subject: [PATCH 5/6] Update chunktransformer.js --- src/chunktransformer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/chunktransformer.js b/src/chunktransformer.js index 70083f2d2..e296870eb 100644 --- a/src/chunktransformer.js +++ b/src/chunktransformer.js @@ -211,7 +211,7 @@ class ChunkTransformer extends Transform { errorMessage = 'A commit happened but the same key followed'; } else if (!chunk.familyName) { errorMessage = 'A family must be set'; - } else if (!chunk.qualifier) { + } else if (null === chunk.qualifier || undefined === chunk.qualifier)= { errorMessage = 'A column qualifier must be set'; } if (errorMessage) { From ca14a399f466ee648ec858b1fab76fa73d08a370 Mon Sep 17 00:00:00 2001 From: Solomon Duskis Date: Thu, 8 Nov 2018 09:28:02 -0500 Subject: [PATCH 6/6] Update chunktransformer.js --- src/chunktransformer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/chunktransformer.js b/src/chunktransformer.js index e296870eb..b1cb209f9 100644 --- a/src/chunktransformer.js +++ b/src/chunktransformer.js @@ -211,7 +211,7 @@ class ChunkTransformer extends Transform { errorMessage = 'A commit happened but the same key followed'; } else if (!chunk.familyName) { errorMessage = 'A family must be set'; - } else if (null === chunk.qualifier || undefined === chunk.qualifier)= { + } else if (chunk.qualifier === null || chunk.qualifier === undefined) { errorMessage = 'A column qualifier must be set'; } if (errorMessage) { @@ -249,7 +249,7 @@ class ChunkTransformer extends Transform { } if ( chunk.familyName && - (null === chunk.qualifier || undefined === chunk.qualifier) + (chunk.qualifier === null || chunk.qualifier === undefined) ) { this.destroy( new TransformError({