fix(cubejs-server-core): fix schema caching in SQL API with DAP on

[bsod90]

Check List

• Tests have been run in packages where changes made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

Issue Reference this PR resolves

[For example #12]

Description of Changes Made (if issue reference is not provided)

[Description goes here]

[mcheshkov]

There's basically zero reason to use MD5 today for any application. I get that there's no performant siphash built in Node.js, but why not at least SHA256? Would probably be faster as well, here's openssl speed output on my laptop:

> openssl speed md5 sha256
...
The 'numbers' are in 1000s of bytes per second processed.
type             16 bytes     64 bytes    256 bytes   1024 bytes   8192 bytes  16384 bytes
md5             140935.16k   353522.47k   701603.62k   932349.94k  1027145.29k  1032898.87k
sha256          193046.72k   585457.47k  1373412.94k  2059078.04k  2404984.89k  2433511.68k

In any case: can you add comment about why this hashing is here, and what are the expectations from hash function?

[bsod90]

md5 is already used in other places in the same file. For this use-case crypto strength isn't important so, any hash would do. Can switch it to sha256

[mcheshkov]

Let's not use XOR mixing, and not do this patching manually, please.

1. One can provide 16 random bytes to uuid.v4 with random option. See link
2. If we want to derive new pseudorandom value deterministically, simplest and safest way would be to just concatenate and hash them again (because we know/expect the length of parts to be fixed): crypto.createHash('sha256').update(uuidBytes).update(hashBytes).digest()

So, why not just uuidv4({random: c.createHash('sha256').update(uuidBytes).update(hashBytes).digest().slice(0,16)})?

[bsod90]

The native side expects compilerId to be a valid uuid (it tries to parse it)

[bsod90]

Also important is that if DAP isn't applied, compilerId stays the same. This is to make sure we don't accidentally introduce any bug in non-DAP use-cases

[mcheshkov]

It will remain valid UUID, that's exactly what uuid.v4 is doing with incoming random bytes. Here. That's what I meant by "not do this patching manually" - keep UUID-related stuff in uuid package.

And compilerId will remain same, because metaConfig will not mix in visibilityMaskHash when it's null.

[bsod90]

Ah, ok. Now I see your point
updated the code

[mcheshkov]

Why slicing just 4 first bytes?

[bsod90]

to make it shorter

[mcheshkov]

I forgot to click "Submit review" yesterday 😞

[mcheshkov]

It will remain valid UUID, that's exactly what uuid.v4 is doing with incoming random bytes. Here. That's what I meant by "not do this patching manually" - keep UUID-related stuff in uuid package.

And compilerId will remain same, because metaConfig will not mix in visibilityMaskHash when it's null.