Conversation
The engine moved twice since this wrapper was written and the suite has been red since. 0.13 added zu_conn_table_name and 0.14 added zu_value_bytes with a type to go with it, and separately two more words became reserved, which broke four cases and one example that had used them as aliases. zu_value_bytes is Value::as_bytes, a span of octets pointing into the result on the same terms as a string: not copied, not NUL terminated, good for as long as the result is. Row::get reads it as that span or as a vector for a caller who wants the octets to outlive the result they came from. Octets and text are separate types both ways, so reading one as the other is refused rather than guessed at, which is the whole point of the engine having a second type: a blob with a zero in it ends early as text and a blob that happens to be valid UTF-8 is still a blob. zu_conn_table_name is Connection::table_name, and it answers nothing rather than failing when no table has that id, because an id no table has is an answer to the question. It copies where the rest of the header borrows, and that is deliberate. The pointer the ABI hands back is good only until the next call of the same function on the same connection, so a string_view over it dangles one line later, and a lifetime nobody can see is worse than an allocation everybody can. It is also the only call on a connection the ABI returns no status for, so the closed handle it would otherwise dereference is caught here instead. The reserved words are big, small and at, and the fix is not to rename the columns. The engine's own message says to write the name in accent quotes, so the aliases are quoted and the columns keep their names, which holds whether or not the grammar reserves another word next month. A property is read by name and needs no quoting; only an alias after AS does.
22 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The suite has been red since the engine moved. Two ABI revisions landed and two more words became reserved, and this covers both.
What the ABI added.
0.13addedzu_conn_table_nameand0.14addedzu_value_byteswithZU_TYPE_BYTESto go with it. The wrapper now has all three, the version check in CMakeLists says 0.14, and the test that asserts the wrapper and the library agree about the ABI passes rather than fails.Value::as_bytesis astd::span<const std::uint8_t>into the result, not copied and not NUL terminated, on exactly the termsas_stringis.Row::getreads it as that span, or as astd::vector<std::uint8_t>for a caller who wants the octets to outlive the result they came out of. Text and octets are separate types in both directions and reading one as the other is refused, which is the point of the engine having a second type at all: a blob with a zero octet in it ends before it starts when read as text, and a blob that happens to be valid UTF-8 is still a blob.Connection::table_nameanswers astd::optional<std::string>, and nothing rather than a failure when no table has that id, because an id no table has is an answer to the question. It copies where the rest of the header borrows. The pointer the ABI hands back is good only until the next call of the same function on the same connection, so a view over it dangles a line later, and a lifetime nobody can see is worse than an allocation everybody can. It is also the one call on a connection the ABI returns no status for, so the closed handle it would otherwise dereference is caught here instead, and there is a case for that.The reserved words are
big,smallandat. The fix is not to rename the columns. The engine's own message says to write the name in accent quotes, so the aliases are quoted and the columns keep the names the cases were written around, which holds whether or not the grammar reserves another word next month. A property is read by name and needs no quoting; only an alias afterASdoes.Verified on a Linux box with gcc 13 against
libzubuilt from engine HEAD, at C++23 and at the C++20 floor: 35 of 35 green, up from 31 of 35. Every claim above was checked against the running engine before it was written down, including that an empty byte string is a byte string rather than a null and that a table id nothing owns comes back as a null pointer.One thing this does not fix. Under
ctest -jthe suite is flaky, and it was before this change: every file is built twice, at C++23 and at the C++20 floor, so two processes run the same cases at once, and the temp directory helper names its directories after the case and a counter that restarts in each process and then clears whatever is already there. The two builds delete each other's databases. That is a separate bug with a separate fix and it is the next PR.