Primary key support #826

ryzhyk · 2023-10-02T19:41:31Z

Support for deleting and updating records by primary key (see #772).

This PR implements upsert update semantics for tables with primary keys, specifically:

An insert into such a table automatically (and quietly) deletes any existing record with the same key.
To delete a record, the client must only specify the value of the key, not the entire record.

TODOs:

Documentation
Support primary keys in JIT.

Is this a user-visible change (yes/no): yes

mihaibudiu · 2023-10-03T23:47:29Z

crates/dbsp/src/operator/input.rs

-            .into_iter()
-            .chain(input_batches().into_iter())
-            .chain(input_batches().into_iter().map(move |batch| {
+        let mut expected_batches = input_batches().into_iter().chain(input_batches()).chain(


the previous version is more readable

mihaibudiu · 2023-10-03T23:51:45Z

crates/dbsp/src/trace/test_batch.rs

@@ -1,6 +1,7 @@
 //! Reference implementation of batch and trace traits for use in testing.
 //!
 //! So far, only methods/traits used in tests have been implemented.
+#![allow(clippy::type_complexity)]


have clippy rules changed?

yes, and I also ran it on parts of the code not previously linted.

mihaibudiu · 2023-10-04T00:05:32Z

crates/adapters/src/static_compile/catalog.rs

+    /// * `KD` - Key type in the input byte stream.  Keys will get deserialized into instances of `KD` and then converted to `K`.
+    /// * `V` - value type of the input collection.
+    /// * `VD` - Value type in the input byte stream.  Values will get deserialized into instances of `VD` and then converted to `K`.
+    pub fn register_input_map<K, KD, V, VD, R, F>(


why is R used for weights?

historical reasons. Inherited it from Frank's code.

mihaibudiu · 2023-10-04T00:29:17Z

crates/adapters/src/static_compile/catalog.rs

+    /// behaves like a map (i.e., has exactly one key with weight 1 per value)
+    /// to the catalog.
+    ///
+    /// Creates neighborhood and quantiles circuits.  Drops the key component


all these extra things should be optional. but I said that already

I agree, and it's not hard to make it optional. We might add this feature soon. Ideally, there would be a way to enable debugging features like this at runtime, but we don't yet have the machinery for this.

crates/dbsp/src/operator/input.rs

Kixiron · 2023-10-04T15:46:38Z

crates/dbsp/src/operator/sample.rs

+    ///
+    /// Equivalent to `self.map(|(k, v)| (k, v)).stream_sample_keys()`,
+    /// but is more efficient.
+    #[allow(clippy::type_complexity)]


We could also just make a type alias for OrdZSet<(B::Key, B::Val), B::R> and reuse it across the various functions

type ZSet<B> = OrdZSet<(B::Key, B::Val), B::R> where B: BatchReader<Time = ()>;

crates/dbsp/src/operator/sample.rs

Kixiron · 2023-10-04T15:50:18Z

crates/dbsp/src/operator/sample.rs

+            let mut builder =
+                <<OrdZSet<_, _> as Batch>::Builder>::with_capacity((), sample_with_vals.len());
+            for key in sample_with_vals.into_iter() {
+                builder.push((key, HasOne::one()));
+            }
+            builder.done()


We can probably optimize this since OrdZSet holds the keys and weights in different vecs, sample_with_vals is already in the proper form, we'd just need to fill another vec with ones (same goes for stream_unique_key_val_quantiles() with some tweaks) and then call OrdZSet::from_columns(sample_with_vals, ones). If the collection we sample from is consolidated then our values should also be sorted, unique and non-zero (and therefore valid zsets)

Kixiron · 2023-10-04T15:51:34Z

crates/dbsp/src/operator/sample.rs

+            trace_handle,
+        ))
+    }
+
    use proptest::{collection::vec, prelude::*};


This use should be at the top of the module

Kixiron · 2023-10-04T15:54:15Z

crates/pipeline_manager/src/integration_test.rs

+            r#"{"insert":{"id":1, "s": "1"}}
+{"insert":{"id":2, "s": "2"}}"#


We could use the json macro + .to_string() for these json literals, could make things nicer (especially the following json literals)

Added a method to register input streams created using `add_input_map` in the catalog. Such a stream represents a table with a unique or primary key and upsert modification semantics. Entries are deleted by key and updated or modified by value (inserting a value whose key already exists in the collection removes the existing value associated with this key). In addition to the input stream, the caller must provide a function to extract keys from values. Signed-off-by: Leonid Ryzhyk <leonid@feldera.com>

Signed-off-by: Leonid Ryzhyk <leonid@feldera.com>

Signed-off-by: Mihai Budiu <mbudiu@gmail.com>

Added an integration test to test primary key tables end-to-end. Signed-off-by: Leonid Ryzhyk <leonid@feldera.com>

ryzhyk · 2023-10-06T18:00:21Z

Thanks, @Kixiron , I applied most of the suggestions.

Signed-off-by: Leonid Ryzhyk <leonid@feldera.com>

ryzhyk changed the title ~~Primary key support~~ [Draft] Primary key support Oct 2, 2023

ryzhyk force-pushed the primary_key branch from 8cb1035 to 1085bbc Compare October 2, 2023 23:31

mihaibudiu mentioned this pull request Oct 3, 2023

Use the new input_set APIs for tables with primary keys #830

Closed

ryzhyk force-pushed the primary_key branch from 1085bbc to 778e64a Compare October 3, 2023 06:59

ryzhyk mentioned this pull request Oct 3, 2023

Map semantics for tables with primary keys. #772

Closed

16 tasks

ryzhyk force-pushed the primary_key branch from 74a4ee7 to 06c9d4f Compare October 3, 2023 20:30

ryzhyk changed the title ~~[Draft] Primary key support~~ Primary key support Oct 3, 2023

ryzhyk requested review from mihaibudiu and Kixiron October 3, 2023 20:35

ryzhyk added SQL compiler Related to the SQL compiler JIT API Distributed system APIs adapters Issues related to the adapters crate User-facing For PRs that lead to Feldera-user visible changes and removed JIT labels Oct 3, 2023

ryzhyk added this to the October 10, 2023 milestone Oct 3, 2023

ryzhyk marked this pull request as ready for review October 3, 2023 20:43

mihaibudiu approved these changes Oct 4, 2023

View reviewed changes

ryzhyk force-pushed the primary_key branch from 06c9d4f to e21bc01 Compare October 4, 2023 06:37

Kixiron approved these changes Oct 4, 2023

View reviewed changes

ryzhyk force-pushed the primary_key branch from e21bc01 to e66e756 Compare October 6, 2023 17:56

Leonid Ryzhyk and others added 8 commits October 6, 2023 10:58

Clippy

fc76f45

Signed-off-by: Leonid Ryzhyk <leonid@feldera.com>

Regression from 4ce7ed.

a621f0f

Signed-off-by: Leonid Ryzhyk <leonid@feldera.com>

Source operators for sets and indexed sets

628c4e7

Signed-off-by: Mihai Budiu <mbudiu@gmail.com>

Convert inputs with primary keys to SourceMapOperators

4597c69

Signed-off-by: Mihai Budiu <mbudiu@gmail.com>

Fix type arguments in new handle APIs

889e714

Signed-off-by: Mihai Budiu <mbudiu@gmail.com>

Fix warning test

c4cb6d9

Signed-off-by: Mihai Budiu <mbudiu@gmail.com>

Integration test for primary keys.

57532c7

Added an integration test to test primary key tables end-to-end. Signed-off-by: Leonid Ryzhyk <leonid@feldera.com>

ryzhyk force-pushed the primary_key branch from e66e756 to 57532c7 Compare October 6, 2023 17:59

Formatting.

a80a52a

Signed-off-by: Leonid Ryzhyk <leonid@feldera.com>

ryzhyk merged commit a462a63 into main Oct 6, 2023
5 checks passed

ryzhyk deleted the primary_key branch October 6, 2023 19:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Primary key support #826

Primary key support #826

ryzhyk commented Oct 2, 2023 •

edited

mihaibudiu Oct 3, 2023

mihaibudiu Oct 3, 2023

ryzhyk Oct 4, 2023

mihaibudiu Oct 4, 2023

ryzhyk Oct 4, 2023

mihaibudiu Oct 4, 2023

ryzhyk Oct 4, 2023

Kixiron Oct 4, 2023

Kixiron Oct 4, 2023 •

edited

Kixiron Oct 4, 2023

Kixiron Oct 4, 2023

ryzhyk commented Oct 6, 2023

		r#"{"insert":{"id":1, "s": "1"}}
		{"insert":{"id":2, "s": "2"}}"#

Primary key support #826

Primary key support #826

Conversation

ryzhyk commented Oct 2, 2023 • edited

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

Kixiron Oct 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryzhyk commented Oct 6, 2023

ryzhyk commented Oct 2, 2023 •

edited

Kixiron Oct 4, 2023 •

edited