-
Notifications
You must be signed in to change notification settings - Fork 12
Add Bolt 6.0 docs #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Looks like you've updated the documentation! Check out your changes at https://neo4j-docs-bolt-80.surge.sh |
Co-authored-by: Stefano Ottolenghi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions after the last round, otherwise good to me! 🤸
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type_marker
type might be wrong.
|
||
[[structure-summary-5]] | ||
[[structure-summary-60]] | ||
=== Version 6.0 | ||
** The <<structure-vector, `Vector`>> structure was added. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to hold to see if we will go with preview release of the protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If team networking prefers to hold back the docs until the protocol has been released into the wild server-side (remember, drivers are already out - albeit in alpha phase), then I'm happy to wait with merging this PR until then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The protocol is there, but it might not be enabled by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Hi. I see you are going to introduce new bolt major version. So far from these changes there is only added one new structure. Does that really need major version increase? Or there is something else not yet documented? I would like to prepared with my php driver. |
Note that bolt's versioning scheme is not semver. Pretty much all protocol changes, even those in minor versions, are breaking. Even though I find myself using the terms major and minor when referring to bolt protocol versions (so do these docs), it's actually a misnomer or at least confusing as it suggest semantic versioning. Instead, we used to align the protocol's "major" version with the DBMS' major version. But now that neo4j has moved to calver there's no longer such thing. Instead, we decided to keep the protocol's major version in sync with the official drivers. It's an attempt at keeping at least some version matrix somewhat simple. Generally, the protocol's version doesn't have much semantic beyond trying to hint at what server and/or driver version it belongs to. |
Based on this, there can be multiple type markers? Or if type_marker is only one for the whole collection of values from data why not make it Integer instead?
|
When data::Bytes contains floating numbers, are these Big endian (as bolt float data type)?
|
No. One marker; all elements inside are of this type and don't come with type markers before each one. Example
🤷 Pretty much arbitrary decision. I suppose an int would've worked as well and would even save 1-2 bytes per vector (which I don't think that's a lot considering vectors are likely going to be rather high-dimensional as they target AI use-cases). On the flip side, an integer might be slightly less intuitive as it will have to be interpreted as a marker byte anyway. Regardless of all of that, this protocol change has been pretty much already implemented across the official drivers, TestKit, and the server side. To now go in and change all of that would probably need a stronger argument than the above.
Yes, as much as the term big-endian makes sense when talking floats. They don't really have a most-significant byte as integers do. Happy to amend the PR to be more explicit here. |
Closes: #77