Skip to content

Commit

Permalink
update to node 16, FIX HASHER BUG (#84)
Browse files Browse the repository at this point in the history
Problem
=======
We need to update to Node16+ as 15.* is @ end-of-life. Closes #62, Fixes
#85, a hasher bug

Solution
========
Update to nodejs 16.15.1, update some packages and fix anything broken.

**Important:** This revealed a bug in the Xxhasher code that was not
converting to hex-encoded strings as the [bloom filter
expects](https://github.com/LibertyDSNP/parquetjs/blob/2c733b5c7a647b9a8ebe9a13d300f41537ef60e2/lib/bloom/sbbf.ts#L357)
  • Loading branch information
shannonwells authored May 25, 2023
1 parent b5698e4 commit 17cb5ed
Show file tree
Hide file tree
Showing 13 changed files with 1,286 additions and 1,688 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- name: Use Node.js
uses: actions/setup-node@v2
with:
node-version: '14.16.0'
node-version: '16.15.1'

- uses: actions/cache@v2
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish-next.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
- name: Use Node.js
uses: actions/setup-node@v2
with:
node-version: '14.16.0'
node-version: '16.15.1'
registry-url: 'https://registry.npmjs.org'

- uses: actions/cache@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
- name: Use Node.js
uses: actions/setup-node@v2
with:
node-version: '14.16.0'
node-version: '16.15.1'
registry-url: 'https://registry.npmjs.org'

- uses: actions/cache@v2
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.DS_STORE
node_modules
*.parquet
npm-debug.log
Expand Down
2 changes: 1 addition & 1 deletion .tool-versions
Original file line number Diff line number Diff line change
@@ -1 +1 @@
nodejs 15.12.0
nodejs 16.15.1
15 changes: 8 additions & 7 deletions lib/bloom/xxhasher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,27 +14,28 @@ type HasherFunc = (input: string, seedHigh?: number, seedLow?: number) => string
* [xxHash spec](https://github.com/Cyan4973/xxHash/blob/v0.7.0/doc/xxhash_spec.md)
*/
export default class XxHasher {
private static h64 = xxhash().then(x => x.h64)
private static h64 = xxhash().then(x => x.h64ToString)

private async hashit(value: string): Promise<string> {
private async hashIt(value: string): Promise<string> {
return (await XxHasher.h64)(value)
}

/**
* @function hash64
* @description creates a hash for certain data types.
* @return the 64 big XXHash as a string
* @param value one of n, throw an error.
* @description creates a hash for certain data types. All data is converted using toString()
* prior to hashing.
* @return the 64 big XXHash as a hex-encoded string.
* @param value, must be of type string, Buffer, Uint8Array, Long, boolean, number, or bigint
*/
async hash64(value: any): Promise<string> {
if (typeof value === 'string') return this.hashit(value)
if (typeof value === 'string') return this.hashIt(value)
if (value instanceof Buffer ||
value instanceof Uint8Array ||
value instanceof Long ||
typeof value === 'boolean' ||
typeof value === 'number' ||
typeof value === 'bigint') {
return this.hashit(value.toString())
return this.hashIt(value.toString())
}
throw new Error("unsupported type: " + value)
}
Expand Down
60 changes: 22 additions & 38 deletions lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,25 +386,34 @@ function fromPrimitive_BSON(value: Buffer) {
return BSON.deserialize(value);
}

function toPrimitive_TIME_MILLIS(value: string | number) {
let v = value
if (typeof value === `string`) {
v = parseInt(value, 10);
function toNumberInternal(typeName: string, value: string | number): number {
let numberValue = 0;
switch (typeof value) {
case "string":
numberValue = parseInt(value, 10);
break;
case "number":
numberValue = value;
break;
default:
throw `${typeName} has an invalid type: ${typeof value}`;
}
// Year 2255 bug. Should eventually switch to bigint
if (v < 0 || v > (Number.MAX_SAFE_INTEGER - 1) || typeof v !== 'number') {
throw 'invalid value for TIME_MILLIS: ' + value;
if (numberValue < 0 || numberValue >= Number.MAX_SAFE_INTEGER) {
throw `${typeName} value is out of bounds: ${numberValue}`;
}
return numberValue
}

return v;
function toPrimitive_TIME_MILLIS(value: string | number) {
return toNumberInternal("TIME_MILLIS", value);
}

function toPrimitive_TIME_MICROS(value: string | number | bigint) {
const v = BigInt(value);
if (v < 0n ) {
throw 'invalid value for TIME_MICROS: ' + value;
throw 'TIME_MICROS value is out of bounds: ' + value;
}

return v;
}

Expand All @@ -415,19 +424,7 @@ function toPrimitive_DATE(value: string | Date | number) {
if (value instanceof Date) {
return value.getTime() / kMillisPerDay;
}

/* convert from integer */
let v = value
if (typeof value === 'string') {
v = parseInt(value, 10);
}

if (v < 0 || typeof v !== 'number') {
throw 'invalid value for DATE: ' + value;
}

return v;

return toNumberInternal("DATE", value )
}

function fromPrimitive_DATE(value: number ) {
Expand All @@ -440,20 +437,7 @@ function toPrimitive_TIMESTAMP_MILLIS(value: string | Date | number) {
if (value instanceof Date) {
return value.getTime();
}

/* convert from integer */

let v = value
if (typeof value === 'string' ) {
v = parseInt(value, 10);
}

if (v < 0 || typeof v !== 'number') {
throw 'invalid value for TIMESTAMP_MILLIS: ' + value;
}

return v;

return toNumberInternal("TIMESTAMP_MILLIS", value);
}

function fromPrimitive_TIMESTAMP_MILLIS(value: number | string | bigint) {
Expand All @@ -471,12 +455,12 @@ function toPrimitive_TIMESTAMP_MICROS(value: Date | string | number | bigint) {
// Will throw if NaN
const v = BigInt(value);
if (v < 0n) {
throw 'Cannot be less than zero';
throw 'out of bounds';
}

return v;
} catch (e) {
throw 'invalid value for TIMESTAMP_MICROS: ' + value;
throw 'TIMESTAMP_MICROS value is out of bounds: ' + value;
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export const osend = function(os: WriteStreamMinimal) {
});
}

export const osopen = function(path: string | Buffer | URL, opts?: string | WriterOptions): Promise<WriteStream> {
export const osopen = function(path: string | Buffer | URL, opts?: WriterOptions): Promise<WriteStream> {
return new Promise((resolve, reject) => {
let outputStream = fs.createWriteStream(path, opts);

Expand Down
Loading

0 comments on commit 17cb5ed

Please sign in to comment.