introduce ndarr support #98

kafka1991 · 2025-04-01T15:04:27Z

introduce ndarr support

TODO Lists

Line protocol version tests in c, c++ and system tests (rust client done)
Rust client's support for common built-in array and vector APIs (for ease of use, less 3 dims)
add more system array tests
Python client support

questdb-rs/src/ingress/mod.rs

amunra · 2025-04-03T17:02:47Z

questdb-rs/src/ingress/ndarr.rs

@@ -0,0 +1,133 @@
+pub(crate) const MAX_DIMS: usize = 32;


I'd move this top-level in questb::ingress and make it public, then add a doc to it. It might be useful to callers.

amunra · 2025-04-04T12:20:16Z

questdb-rs/src/error.rs

+    /// Buffer outOfMemory.
+    BufferOutOfMemory,


questdb-rs/src/ingress/ndarr.rs

amunra · 2025-04-04T12:30:23Z

questdb-rs/src/ingress/mod.rs

@@ -24,8 +24,8 @@

 #![doc = include_str!("mod.md")]

+pub use self::ndarr::*;


The * risks leaking too many types.
Instead just pick the ones you explicitly are sure you want in the public API.

questdb-rs/src/tests/mock.rs

amunra · 2025-04-04T12:37:59Z

questdb-rs/src/tests/mod.rs

@@ -29,6 +29,9 @@ mod http;
 mod mock;
 mod sender;

+#[cfg(feature = "ndarray")]
+mod ndarr;


There should be tests for the trait directly as well, even without ndarray enabled.

This is to be sure that we never tie the ndarray feature to the ability to write arrays.

The tests that check the NdArrayView trait should also check what happens when the trait does nothing or less data.

Another test to write is if the trait gives back the wrong number of dimensions.

amunra · 2025-04-04T12:44:26Z

include/questdb/ingress/line_sender.hpp

+    template <typename T, size_t N>
+    line_sender_buffer& column(
+        column_name_view name,
+        const std::vector<uint32_t>& shape,
+        const std::array<T, N>& data)


We need to come up with something that's a bit more idiomatic C++.
Let's park the C++ API for now until we have everything else nailed down.

questdb-rs/src/ingress/ndarr.rs

# Conflicts: # questdb-rs/Cargo.toml

amunra · 2025-05-13T17:10:26Z

questdb-rs/src/ingress/mod.rs

+        self.f64serializer = match version {
+            LineProtocolVersion::V1 => f64_text_series,
+            LineProtocolVersion::V2 => f64_binary_series,
+        };


Generally function dispatch is actually a fair bit slower than a branch.
Remove this field and just use a match on the protocol.

You could bias the V2 behaviour with std::hint::likely.
Inlining the f64 serialization functions is probably a good idea too.

amunra · 2025-05-13T17:18:24Z

questdb-rs/src/ingress/mod.rs

@@ -953,8 +1012,7 @@ impl Buffer {
        Error: From<N::Error>,
    {
        self.write_column_key(name)?;
-        let mut ser = F64Serializer::new(value);
-        self.output.extend_from_slice(ser.as_str().as_bytes());
+        (self.f64serializer)(&mut self.output, value);


if likely(!matches!(version, LineProtocolVersion::V1)) { // Protocols V2+ binary } else { // Protocol V1 text }

amunra · 2025-05-13T17:19:56Z

questdb-rs/src/ingress/mod.rs

+
+/// Line Protocol Version supported by current client.
+#[derive(Debug, Copy, Clone, PartialEq)]
+pub enum LineProtocolVersion {


Rename to ProtocolVersion to match what I think is a better config setting, I.e. protocol_version=1|2|auto.

amunra · 2025-05-13T17:21:45Z

questdb-rs/src/ingress/mod.rs

+    /// # {
+    /// # use questdb::Result;
+    /// # use questdb::ingress::Buffer;
+    /// # use ndarray::array;


Don't comment the use ndarray::array line here. Should be shown as part of the example.

amunra · 2025-05-13T17:22:13Z

questdb-rs/src/ingress/mod.rs

+    /// Record a multidimensional array value for the given column.
+    ///
+    /// Supports arrays with up to [`MAX_ARRAY_DIMS`] dimensions. The array elements must
+    /// implement [`ArrayElement`] trait which provides type-to-[`ElemDataType`] mapping.


ElemDataType has been renamed now.

Probably worth listing all the supported datatypes here so users don't need to hunt around in the documentation.

amunra · 2025-05-13T17:24:18Z

questdb-rs/src/ingress/mod.rs

+    /// Using [`ColumnName`] for validated column names:
+    ///
+    /// ```
+    /// # #[cfg(feature = "ndarray")]
+    /// # {
+    /// # use questdb::Result;
+    /// # use questdb::ingress::{Buffer, ColumnName};
+    /// # use ndarray::Array3;
+    /// # fn main() -> Result<()> {
+    /// # let mut buffer = Buffer::new();
+    /// # buffer.table("x1")?;
+    /// // Record a 3D array of f64 values
+    /// let array_3d = Array3::from_elem((2, 3, 4), 42f64);
+    /// let col_name = ColumnName::new("col1")?;
+    /// buffer.column_arr(col_name, &array_3d.view())?;
+    /// # Ok(())
+    /// # }
+    /// # }


I'd remove this example and show one with slices and another with vectors.

You can use some headings here.

Run

cargo doc --open

To preview.

amunra · 2025-05-13T17:32:02Z

questdb-rs/src/ingress/mod.rs

@@ -1199,6 +1426,9 @@ pub struct Sender {
    handler: ProtocolHandler,
    connected: bool,
    max_buf_size: usize,
+    default_line_protocol_version: LineProtocolVersion,
+    #[cfg(feature = "ilp-over-http")]
+    supported_line_protocol_versions: Option<Vec<LineProtocolVersion>>,


Probably worth commenting why it's an Option<Vec<..>> and not just a Vec<..>.

I'm assuming that's because it's None if no probing was done.
I'm just afraid neither of us will rememember in 6 months time.

amunra · 2025-05-13T17:34:16Z

questdb-rs/src/ingress/mod.rs

+    ///
+    /// Please ensure client's default version ([`LINE_PROTOCOL_VERSION_V2`]) or
+    /// explicitly set protocol version exactly matches server expectation.
+    pub fn disable_line_protocol_validation(mut self) -> Result<Self> {


Replace with pub fn protocol_version(mut self, vers: ProtocolVersion), overriding the default when called.

amunra · 2025-05-13T17:35:42Z

questdb-rs/src/ingress/mod.rs

@@ -1199,6 +1426,9 @@ pub struct Sender {
    handler: ProtocolHandler,
    connected: bool,
    max_buf_size: usize,
+    default_line_protocol_version: LineProtocolVersion,


Replace with protocol_version: Option<ProtocolVersion> where None means "auto".

amunra · 2025-05-13T17:39:31Z

questdb-rs/src/ingress/http.rs

+pub(super) fn get_line_protocol_version(
+    state: &HttpHandlerState,
+    settings_url: &str,
+) -> Result<(Option<Vec<LineProtocolVersion>>, LineProtocolVersion), Error> {


Document return type: I would have expected a single Vec<ProtocolVersion>.

Also ensure that there's tests for the following scenarios:

The server returns 404.

Default to V1

The server returns, but the field is missing from the JSON.

Default to V1

The server returns only version 1 (if the v2 protocol gets toggled via config in the server).

The server returns [1, 2].

Client uses V2.

The server returns [2].

Client uses V2.

The server returns [MAX_SUPPORTED_VERSION + 1].

The client errors, since there's no compatible protocol.

For these tests, ensure create a buffer from server.new_buffer(), add a simple with just a timestamp and an f64 and flush it, checking the serialized behaviour.

amunra · 2025-05-13T17:46:07Z

questdb-rs/src/error.rs

+    ArrayWriteToBufferError,
+
+    /// Validate line protocol version error.
+    LineProtocolVersionError,


Rename to ProtocolVersionError.

amunra · 2025-05-13T17:47:15Z

questdb-rs/Cargo.toml

@@ -83,6 +86,8 @@ json_tests = []
 # Enable methods to create timestamp objects from chrono::DateTime objects.
 chrono_timestamp = ["chrono"]

+benchmark = []


amunra · 2025-05-13T17:47:20Z

questdb-rs/Cargo.toml

@@ -55,6 +56,8 @@ mio = { version = "1", features = ["os-poll", "net"] }
 chrono = "0.4.31"
 tempfile = "3"
 webpki-roots = "0.26.8"
+criterion = "0.5"


…ng, since we are no longer compatible

…nto array

c client nd array begin.

87aa240

kafka1991 self-assigned this Apr 1, 2025

kafka1991 marked this pull request as draft April 1, 2025 15:04

kafka1991 added 10 commits April 2, 2025 19:26

Ndarray support for rust client.

6bd3223

optimize doc.

b518a99

add tests.

53a430e

fix comments.

b1ebd1d

fix docs.

006af98

fix tests and compile error in ffi.

a31716a

format code.

463a2d3

ffi supported

44fae08

ndarray c api

93c6f9d

format

e9da558

kafka1991 marked this pull request as ready for review April 3, 2025 09:05

kafka1991 added 3 commits April 3, 2025 21:36

c++ api support.

0e8a9e5

add include.

261163c

code format.

a565109

amunra reviewed Apr 3, 2025

View reviewed changes

questdb-rs/src/ingress/mod.rs Outdated Show resolved Hide resolved

amunra reviewed Apr 3, 2025

View reviewed changes

questdb-rs/src/ingress/mod.rs Outdated Show resolved Hide resolved

amunra reviewed Apr 3, 2025

View reviewed changes

amunra reviewed Apr 4, 2025

View reviewed changes

questdb-rs/src/ingress/ndarr.rs Outdated Show resolved Hide resolved

amunra reviewed Apr 4, 2025

View reviewed changes

questdb-rs/src/tests/mock.rs Outdated Show resolved Hide resolved

amunra reviewed Apr 4, 2025

View reviewed changes

questdb-rs/src/ingress/ndarr.rs Outdated Show resolved Hide resolved

kafka1991 added 3 commits April 6, 2025 22:21

Merge remote-tracking branch 'origin/main' into array

d1edbbc

# Conflicts: # questdb-rs/Cargo.toml

conflict resolve

e161b1d

code review.

4a59ec0

amunra reviewed May 13, 2025

View reviewed changes

kafka1991 and others added 9 commits May 14, 2025 21:19

code review.

b89788c

fix c api

58c6539

code reviews.

8fb0c4a

rename c api new_buffer() name.

56e349a

Merge remote-tracking branch 'origin/main' into array

ce8a084

fix system.tests

f163a31

Merge remote-tracking branch 'origin/array' into array

82b0dc3

renamed Influx to Ingestion in various places to avoid being misleadi…

0c5e509

…ng, since we are no longer compatible

fix systemtests

6e1e68d

kafka1991 force-pushed the array branch from 8be5784 to 6e1e68d Compare May 15, 2025 12:47

amunra and others added 7 commits May 15, 2025 15:40

reworked the python system tests for cross version testing

a461bfd

enrich rust tests.

bf419f1

clearing all tables before each test run

01bfd91

Merge branch 'array' of https://github.com/questdb/c-questdb-client i…

36cb2e3

…nto array

fixed broken C example

8c1d9b9

minor fixes

36f4bd5

fix abnormal tests hang.

3392627

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduce ndarr support #98

introduce ndarr support #98

kafka1991 commented Apr 1, 2025 •

edited

Loading

amunra Apr 3, 2025

kafka1991 Apr 7, 2025

amunra Apr 4, 2025

kafka1991 Apr 7, 2025

amunra Apr 4, 2025

amunra Apr 4, 2025

amunra Apr 4, 2025

amunra Apr 4, 2025

amunra May 13, 2025

amunra May 13, 2025 •

edited

Loading

amunra May 13, 2025 •

edited

Loading

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

amunra May 13, 2025

		@@ -24,8 +24,8 @@

		#![doc = include_str!("mod.md")]

		pub use self::ndarr::*;

introduce ndarr support #98

Are you sure you want to change the base?

introduce ndarr support #98

Conversation

kafka1991 commented Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amunra May 13, 2025 • edited Loading

Choose a reason for hiding this comment

amunra May 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kafka1991 commented Apr 1, 2025 •

edited

Loading

amunra May 13, 2025 •

edited

Loading

amunra May 13, 2025 •

edited

Loading