Difficulties using TAIL from a NodeJS backend

[petrosagg]

I encountered the following issues trying to build a realtime application based on TAIL:

• TAIL outputs rows that can have conflicting names if the target view contains columns named timestamp, diff, or progressed
  Timestamp in particular is a pretty common name. Libraries that auto convert rows into hashmaps (like node-postgres) will only keep one of the two. While it is possible to use alternative APIs and access the row as an array, it's cumbersome.
  Proposal: We could choose less likely names for the system columns like mz_timestamp, mz_diff, and mz_progressed

• My application was listening on the TAIL stream and was reconstructing the collection according to the diffs. I wanted to only release well-formed collections to the rest of the application so the receiving code needed to know when a timestamp is complete. TAIL doesn't provide a consistent mechanism for that, even when you do WITH (PROGRESS). The timestamps progress if you get a row where it has progressed=true OR if you receive a row with a timestamp higher than the previous row. This also implies that data might be buffered on the materialize side to make this happen.
  Proposal: Always send progress messages when a timestamp closes and don't hold back any row data, at least in the WITH (PROGRESS) mode

• The diff field is declared as a i64 which makes the javascript client to provide it as a string since JS only has 64bit floats.
  Proposal: Make the diff field i32. It's extremely unlikely that any system will produce a single row with cardinality over 2 billion

[benesch]

Point 1 is #5908, and the proposal you suggest here is the one agreed upon by everyone!

cc @mjibson for thoughts on point 2.

Re point 3: I'm really loathe on the Materialize side to do anything to work around JS's numeric woes. We had this problem at Cockroach too, and, well, folks who use PostgreSQL seriously from JS are used to dealing with this, and usually have some workarounds. JS is slowly getting a BigInt type that may one day be supported by node-pg: brianc/node-pg-types#78. In the meantime you can use pg.defaults.parseInt8(true) to YOLO it if you're sure you'll never see an int8 that exceeds the maximum safe integer.

[petrosagg]

oh, great about #5908! amazing that we ended up in an identical proposal

re point 3: If the rule is "we don't cut corners because of JS", then I fully agree with it and I will argue that this change does not violate it. The argument that i64 is excessive for the diff field I think stands on its own regardless of the minor JS inconvenience that made me realize it. I also saw that repr::Diffis defined as isize in the code, is that on purpose?

In my demo I did the even YOLOer thing of using == instead of === to let JS do its type coercion abominations :P

[philip-stoev]

Every time a 32-bit has been used for anything internal a the database system it has overflowed and had to be changed to a 64-bit integer. record IDs, message IDs, anything , always. I suggest we keep this a 64-bit thing.

If you have a source with 2 billion NULLs in it, you can potentially get a diff with such a large value if you are very unlucky.

[petrosagg]

Alright, I'm convinced. I withdraw point 3 :)

[maddyblue]

I thought that progress=true produced periodic messages even if there were no rows. There's no code that buffers in the "after dataflow" side. We even specifically care about the case where there's no rows and thus just a progress event

materialize/src/dataflow/src/sink/tail.rs

Line 77 in 4d68d9e

// batch. All of the batches might have zero rows, so we do not depend on

. So I'm not sure what's going on there? It might be a dataflow-side thing which I don't know about.

[benesch]

Adding to To Do in both SQL and Coordinator and Sources and Sinks so we don't lose track of this. There's definitely something strange here. @petrosagg, would you be able to provide repro instructions for the missing progress messages?

[maddyblue]

This is easily reproducible though:

materialize=> create table t (i int);
CREATE TABLE
materialize=> copy (tail t with (progress=true)) to stdout;

has no output, when I would expect it to. It perhaps did at one point and this regressed.

[maddyblue]

I was not able to produce empty progress messages for any version since this was released in 0.5.2 so I assume I misremember having them ever. We just wrote the code such that they might exist, but they never did.

[maddyblue]

I've opened #7257 to track the empty progress messages.

[benesch]

Ok, sounds like #7257 covers it then!