Skip to content

Commit

Permalink
Adds PartiqlShapeBuilder with NodeId generation for the `StaticTy…
Browse files Browse the repository at this point in the history
…pe` (#485)

* Adds `NodeId` to the `StaticType`

This PR
- adds `NodeId` to `StaticType`; this is to be able to use the `id` as a reference to add additional data to the types out of band.
- makes `AutoNodeIdGenerator` thread-safe
- adds `PartiqlShapeBuilder` and moves some `PartiqlShape` APIs to it; this is to be able to generate unique `NodeId`s for a `PartiqlShape` that includes static types that themselves can include other static types.
- adds a static thread safe `shape_builder` function that provides a convenient way for using `PartiqlShapeBuilder` for creating new shapes.
- prepends existing type macros with `type` such as `type_int!` to make macro names more friendly.
- removes `const` PartiQL types under `partiql-types` in favor of `PartiqlShapeBuilder`.

**_The majority of the diffs are related to the macro renames or replacement with the previous `const` types. The main change is in `partiql-types/src/lib.rs` file._**
  • Loading branch information
am357 authored Aug 14, 2024
1 parent e5b8ce7 commit 7203852
Show file tree
Hide file tree
Showing 16 changed files with 714 additions and 483 deletions.
2 changes: 1 addition & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[submodule "partiql-conformance-tests/partiql-tests"]
path = partiql-conformance-tests/partiql-tests
url = git@github.com:partiql/partiql-tests.git
url = https://github.com/partiql/partiql-tests.git
13 changes: 12 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- partiql-ast: improved pretty-printing of `CASE` and various clauses
-
### Added
- Added `partiql-common`.
- Added `NodeId` to `StaticType`.
- *BREAKING* Added thread-safe `PartiqlShapeBuilder` and automatic `NodeId` generation for the `StaticType`.
- Added a static thread safe `shape_builder` function that provides a convenient way for using `PartiqlShapeBuilder` for creating new shapes.

### Fixed
### Removed
- *BREAKING* Removed `partiql-source-map`.
- *BREAKING* Removed `const` PartiQL types under `partiql-types` in favor of `PartiqlShapeBuilder`.
- *BREAKING* Removed `StaticType`'s `new`, `new_non_nullable`, and `as_non-nullable` APIs in favor of `PartiqlShapeBuilder`.

## [0.10.0]
### Changed
- *BREAKING:* partiql-ast: added modeling of `EXCLUDE`
- *BREAKING:* partiql-ast: added pretty-printing of `EXCLUDE`
- *BREAKING* Moved some of the `PartiqlShape` APIs to the `PartiqlShapeBuilder`.
- *BREAKING* Prepended existing type macros with `type` to make macro names more friendly: e.g., `type_int!`
- *BREAKING* Moved node id generation and `partiql-source-map` to it.
- *BREAKING* Changed `AutoNodeIdGenerator` to a thread-safe version

### Added
- *BREAKING:* partiql-parser: added parsing of `EXCLUDE`
Expand Down
58 changes: 35 additions & 23 deletions extension/partiql-extension-ddl/src/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ impl PartiqlBasicDdlEncoder {
Static::Float64 => out.push_str("DOUBLE"),
Static::String => out.push_str("VARCHAR"),
Static::Struct(s) => out.push_str(&self.write_struct(s)?),
Static::Bag(b) => out.push_str(&self.write_bag(b)?),
Static::Array(a) => out.push_str(&self.write_array(a)?),
Static::Bag(b) => out.push_str(&self.write_type_bag(b)?),
Static::Array(a) => out.push_str(&self.write_type_array(a)?),
// non-exhaustive catch-all
_ => todo!("handle type for {}", ty),
}
Expand All @@ -136,12 +136,18 @@ impl PartiqlBasicDdlEncoder {
Ok(out)
}

fn write_bag(&self, bag: &BagType) -> ShapeDdlEncodeResult<String> {
Ok(format!("BAG<{}>", self.write_shape(bag.element_type())?))
fn write_type_bag(&self, type_bag: &BagType) -> ShapeDdlEncodeResult<String> {
Ok(format!(
"type_bag<{}>",
self.write_shape(type_bag.element_type())?
))
}

fn write_array(&self, arr: &ArrayType) -> ShapeDdlEncodeResult<String> {
Ok(format!("ARRAY<{}>", self.write_shape(arr.element_type())?))
fn write_type_array(&self, arr: &ArrayType) -> ShapeDdlEncodeResult<String> {
Ok(format!(
"type_array<{}>",
self.write_shape(arr.element_type())?
))
}

fn write_struct(&self, strct: &StructType) -> ShapeDdlEncodeResult<String> {
Expand Down Expand Up @@ -189,8 +195,8 @@ impl PartiqlDdlEncoder for PartiqlBasicDdlEncoder {
let mut output = String::new();
let ty = ty.expect_static()?;

if let Static::Bag(bag) = ty.ty() {
let s = bag.element_type().expect_struct()?;
if let Static::Bag(type_bag) = ty.ty() {
let s = type_bag.element_type().expect_struct()?;
let mut fields = s.fields().peekable();
while let Some(field) = fields.next() {
output.push_str(&format!("\"{}\" ", field.name()));
Expand Down Expand Up @@ -223,41 +229,47 @@ impl PartiqlDdlEncoder for PartiqlBasicDdlEncoder {
mod tests {
use super::*;
use indexmap::IndexSet;
use partiql_types::{array, bag, f64, int8, r#struct, str, struct_fields, StructConstraint};
use partiql_types::{
struct_fields, type_array, type_bag, type_float64, type_int8, type_string, type_struct,
PartiqlShapeBuilder, StructConstraint,
};

#[test]
fn ddl_test() {
let nested_attrs = struct_fields![
(
"a",
PartiqlShape::any_of(vec![
PartiqlShape::new(Static::DecimalP(5, 4)),
PartiqlShape::new(Static::Int8),
PartiqlShapeBuilder::init_or_get().any_of(vec![
PartiqlShapeBuilder::init_or_get().new_static(Static::DecimalP(5, 4)),
PartiqlShapeBuilder::init_or_get().new_static(Static::Int8),
])
),
("b", array![str![]]),
("c", f64!()),
("b", type_array![type_string![]]),
("c", type_float64!()),
];
let details = r#struct![IndexSet::from([nested_attrs])];
let details = type_struct![IndexSet::from([nested_attrs])];

let fields = struct_fields![
("employee_id", int8![]),
("full_name", str![]),
("salary", PartiqlShape::new(Static::DecimalP(8, 2))),
("employee_id", type_int8![]),
("full_name", type_string![]),
(
"salary",
PartiqlShapeBuilder::init_or_get().new_static(Static::DecimalP(8, 2))
),
("details", details),
("dependents", array![str![]])
("dependents", type_array![type_string![]])
];
let ty = bag![r#struct![IndexSet::from([
let ty = type_bag![type_struct![IndexSet::from([
fields,
StructConstraint::Open(false)
])]];

let expected_compact = r#""employee_id" TINYINT,"full_name" VARCHAR,"salary" DECIMAL(8, 2),"details" STRUCT<"a": UNION<DECIMAL(5, 4),TINYINT>,"b": ARRAY<VARCHAR>,"c": DOUBLE>,"dependents" ARRAY<VARCHAR>"#;
let expected_compact = r#""employee_id" TINYINT,"full_name" VARCHAR,"salary" DECIMAL(8, 2),"details" STRUCT<"a": UNION<DECIMAL(5, 4),TINYINT>,"b": type_array<VARCHAR>,"c": DOUBLE>,"dependents" type_array<VARCHAR>"#;
let expected_pretty = r#""employee_id" TINYINT,
"full_name" VARCHAR,
"salary" DECIMAL(8, 2),
"details" STRUCT<"a": UNION<DECIMAL(5, 4),TINYINT>,"b": ARRAY<VARCHAR>,"c": DOUBLE>,
"dependents" ARRAY<VARCHAR>"#;
"details" STRUCT<"a": UNION<DECIMAL(5, 4),TINYINT>,"b": type_array<VARCHAR>,"c": DOUBLE>,
"dependents" type_array<VARCHAR>"#;

let ddl_compact = PartiqlBasicDdlEncoder::new(DdlFormat::Compact);
assert_eq!(ddl_compact.ddl(&ty).expect("write shape"), expected_compact);
Expand Down
22 changes: 14 additions & 8 deletions extension/partiql-extension-ddl/tests/ddl-tests.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
use indexmap::IndexSet;
use partiql_extension_ddl::ddl::{DdlFormat, PartiqlBasicDdlEncoder, PartiqlDdlEncoder};
use partiql_types::{bag, int, r#struct, str, struct_fields, StructConstraint, StructField};
use partiql_types::{BagType, PartiqlShape, Static, StructType};
use partiql_types::{
struct_fields, type_bag, type_int, type_string, type_struct, PartiqlShapeBuilder,
StructConstraint, StructField,
};
use partiql_types::{BagType, Static, StructType};

#[test]
fn basic_ddl_test() {
let details_fields = struct_fields![("age", int!())];
let details = r#struct![IndexSet::from([details_fields])];
let details_fields = struct_fields![("age", type_int!())];
let details = type_struct![IndexSet::from([details_fields])];
let fields = [
StructField::new("id", int!()),
StructField::new("name", str!()),
StructField::new("address", PartiqlShape::new_non_nullable(Static::String)),
StructField::new("id", type_int!()),
StructField::new("name", type_string!()),
StructField::new(
"address",
PartiqlShapeBuilder::init_or_get().new_non_nullable_static(Static::String),
),
StructField::new_optional("details", details.clone()),
]
.into();
let shape = bag![r#struct![IndexSet::from([
let shape = type_bag![type_struct![IndexSet::from([
StructConstraint::Fields(fields),
StructConstraint::Open(false)
])]];
Expand Down
3 changes: 2 additions & 1 deletion partiql-ast/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ where

pub fn node<T>(&mut self, node: T) -> AstNode<T> {
let id = self.id_gen.id();
AstNode { id, node }
let id = id.read().expect("NodId read lock");
AstNode { id: *id, node }
}
}

Expand Down
31 changes: 23 additions & 8 deletions partiql-common/src/node.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use indexmap::IndexMap;
use std::hash::Hash;
use std::sync::{Arc, RwLock};

#[cfg(feature = "serde")]
use serde::{Deserialize, Serialize};
Expand All @@ -12,27 +13,37 @@ pub struct NodeId(pub u32);

/// Auto-incrementing [`NodeIdGenerator`]
pub struct AutoNodeIdGenerator {
next_id: NodeId,
next_id: Arc<RwLock<NodeId>>,
}

impl Default for AutoNodeIdGenerator {
fn default() -> Self {
AutoNodeIdGenerator { next_id: NodeId(1) }
AutoNodeIdGenerator {
next_id: Arc::new(RwLock::from(NodeId(1))),
}
}
}

/// A provider of 'fresh' [`NodeId`]s.
pub trait NodeIdGenerator {
fn id(&self) -> Arc<RwLock<NodeId>>;

/// Provides a 'fresh' [`NodeId`].
fn id(&mut self) -> NodeId;
fn next_id(&self) -> NodeId;
}

impl NodeIdGenerator for AutoNodeIdGenerator {
fn id(&self) -> Arc<RwLock<NodeId>> {
let id = self.next_id();
let mut w = self.next_id.write().expect("NodId write lock");
*w = id;
Arc::clone(&self.next_id)
}

#[inline]
fn id(&mut self) -> NodeId {
let mut next = NodeId(&self.next_id.0 + 1);
std::mem::swap(&mut self.next_id, &mut next);
next
fn next_id(&self) -> NodeId {
let id = &self.next_id.read().expect("NodId read lock");
NodeId(id.0 + 1)
}
}

Expand All @@ -41,7 +52,11 @@ impl NodeIdGenerator for AutoNodeIdGenerator {
pub struct NullIdGenerator {}

impl NodeIdGenerator for NullIdGenerator {
fn id(&mut self) -> NodeId {
fn id(&self) -> Arc<RwLock<NodeId>> {
Arc::new(RwLock::from(self.next_id()))
}

fn next_id(&self) -> NodeId {
NodeId(0)
}
}
4 changes: 2 additions & 2 deletions partiql-eval/src/eval/eval_expr_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::eval::expr::{BindError, EvalExpr};
use crate::eval::EvalContext;
use itertools::Itertools;

use partiql_types::{PartiqlShape, Static, TYPE_DYNAMIC};
use partiql_types::{type_dynamic, PartiqlShape, Static, TYPE_DYNAMIC};
use partiql_value::Value::{Missing, Null};
use partiql_value::{Tuple, Value};

Expand Down Expand Up @@ -413,7 +413,7 @@ impl UnaryValueExpr {
where
F: 'static + Fn(&Value) -> Value,
{
Self::create_typed::<STRICT, F>([TYPE_DYNAMIC; 1], args, f)
Self::create_typed::<STRICT, F>([type_dynamic!(); 1], args, f)
}

#[allow(dead_code)]
Expand Down
30 changes: 17 additions & 13 deletions partiql-eval/src/eval/expr/coll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use crate::eval::expr::{BindError, BindEvalExpr, EvalExpr};

use itertools::{Itertools, Unique};

use partiql_types::{ArrayType, BagType, PartiqlShape, Static, TYPE_BOOL, TYPE_NUMERIC_TYPES};
use partiql_types::{
type_bool, type_numeric, ArrayType, BagType, PartiqlShape, PartiqlShapeBuilder, Static,
};
use partiql_value::Value::{Missing, Null};
use partiql_value::{BinaryAnd, BinaryOr, Value, ValueIter};

Expand Down Expand Up @@ -49,21 +51,23 @@ impl BindEvalExpr for EvalCollFn {
value.sequence_iter().map_or(Missing, &f)
})
}
let boolean_elems = [PartiqlShape::any_of([
PartiqlShape::new(Static::Array(ArrayType::new(Box::new(TYPE_BOOL)))),
PartiqlShape::new(Static::Bag(BagType::new(Box::new(TYPE_BOOL)))),
let boolean_elems = [PartiqlShapeBuilder::init_or_get().any_of([
PartiqlShapeBuilder::init_or_get()
.new_static(Static::Array(ArrayType::new(Box::new(type_bool!())))),
PartiqlShapeBuilder::init_or_get()
.new_static(Static::Bag(BagType::new(Box::new(type_bool!())))),
])];
let numeric_elems = [PartiqlShape::any_of([
PartiqlShape::new(Static::Array(ArrayType::new(Box::new(
PartiqlShape::any_of(TYPE_NUMERIC_TYPES),
let numeric_elems = [PartiqlShapeBuilder::init_or_get().any_of([
PartiqlShapeBuilder::init_or_get().new_static(Static::Array(ArrayType::new(Box::new(
PartiqlShapeBuilder::init_or_get().any_of(type_numeric!()),
)))),
PartiqlShapeBuilder::init_or_get().new_static(Static::Bag(BagType::new(Box::new(
PartiqlShapeBuilder::init_or_get().any_of(type_numeric!()),
)))),
PartiqlShape::new(Static::Bag(BagType::new(Box::new(PartiqlShape::any_of(
TYPE_NUMERIC_TYPES,
))))),
])];
let any_elems = [PartiqlShape::any_of([
PartiqlShape::new(Static::Array(ArrayType::new_any())),
PartiqlShape::new(Static::Bag(BagType::new_any())),
let any_elems = [PartiqlShapeBuilder::init_or_get().any_of([
PartiqlShapeBuilder::init_or_get().new_static(Static::Array(ArrayType::new_any())),
PartiqlShapeBuilder::init_or_get().new_static(Static::Bag(BagType::new_any())),
])];

match *self {
Expand Down
4 changes: 2 additions & 2 deletions partiql-eval/src/eval/expr/datetime.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::eval::expr::{BindError, BindEvalExpr, EvalExpr};

use partiql_types::TYPE_DATETIME;
use partiql_types::type_datetime;
use partiql_value::Value::Missing;
use partiql_value::{DateTime, Value};

Expand Down Expand Up @@ -43,7 +43,7 @@ impl BindEvalExpr for EvalExtractFn {
}

let create = |f: fn(&DateTime) -> Value| {
UnaryValueExpr::create_typed::<{ STRICT }, _>([TYPE_DATETIME], args, move |value| {
UnaryValueExpr::create_typed::<{ STRICT }, _>([type_datetime!()], args, move |value| {
match value {
Value::DateTime(dt) => f(dt.as_ref()),
_ => Missing,
Expand Down
Loading

1 comment on commit 7203852

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PartiQL (rust) Benchmark

Benchmark suite Current: 7203852 Previous: e5b8ce7 Ratio
arith_agg-avg 761989 ns/iter (± 2584) 775198 ns/iter (± 34461) 0.98
arith_agg-avg_distinct 856629 ns/iter (± 2352) 856544 ns/iter (± 5154) 1.00
arith_agg-count 813071 ns/iter (± 16235) 820861 ns/iter (± 18988) 0.99
arith_agg-count_distinct 849796 ns/iter (± 3499) 852867 ns/iter (± 4970) 1.00
arith_agg-min 817287 ns/iter (± 12938) 827041 ns/iter (± 2639) 0.99
arith_agg-min_distinct 854925 ns/iter (± 1637) 860210 ns/iter (± 10560) 0.99
arith_agg-max 828374 ns/iter (± 15165) 835593 ns/iter (± 3961) 0.99
arith_agg-max_distinct 863610 ns/iter (± 3061) 864949 ns/iter (± 4270) 1.00
arith_agg-sum 817340 ns/iter (± 4491) 824926 ns/iter (± 1621) 0.99
arith_agg-sum_distinct 857382 ns/iter (± 5483) 857067 ns/iter (± 1983) 1.00
arith_agg-avg-count-min-max-sum 962352 ns/iter (± 2579) 968375 ns/iter (± 8380) 0.99
arith_agg-avg-count-min-max-sum-group_by 1184994 ns/iter (± 37004) 1204293 ns/iter (± 13453) 0.98
arith_agg-avg-count-min-max-sum-group_by-group_as 1802258 ns/iter (± 23102) 1801364 ns/iter (± 31255) 1.00
arith_agg-avg_distinct-count_distinct-min_distinct-max_distinct-sum_distinct 1258025 ns/iter (± 50557) 1229000 ns/iter (± 13263) 1.02
arith_agg-avg_distinct-count_distinct-min_distinct-max_distinct-sum_distinct-group_by 1517674 ns/iter (± 8829) 1508448 ns/iter (± 9427) 1.01
arith_agg-avg_distinct-count_distinct-min_distinct-max_distinct-sum_distinct-group_by-group_as 2111719 ns/iter (± 14340) 2098733 ns/iter (± 10245) 1.01
parse-1 5949 ns/iter (± 77) 5619 ns/iter (± 23) 1.06
parse-15 49885 ns/iter (± 225) 48240 ns/iter (± 118) 1.03
parse-30 100431 ns/iter (± 280) 93707 ns/iter (± 250) 1.07
compile-1 4345 ns/iter (± 30) 4326 ns/iter (± 10) 1.00
compile-15 33556 ns/iter (± 98) 33011 ns/iter (± 270) 1.02
compile-30 69779 ns/iter (± 296) 68472 ns/iter (± 296) 1.02
plan-1 67795 ns/iter (± 269) 66983 ns/iter (± 261) 1.01
plan-15 1047210 ns/iter (± 9506) 1047620 ns/iter (± 5242) 1.00
plan-30 2098549 ns/iter (± 5644) 2101187 ns/iter (± 26131) 1.00
eval-1 12755601 ns/iter (± 45848) 13463456 ns/iter (± 113930) 0.95
eval-15 86481769 ns/iter (± 851403) 91368497 ns/iter (± 276047) 0.95
eval-30 166749677 ns/iter (± 551437) 174268173 ns/iter (± 529438) 0.96
join 9733 ns/iter (± 210) 9998 ns/iter (± 152) 0.97
simple 2478 ns/iter (± 76) 2479 ns/iter (± 11) 1.00
simple-no 435 ns/iter (± 1) 428 ns/iter (± 5) 1.02
numbers 57 ns/iter (± 0) 57 ns/iter (± 0) 1
parse-simple 901 ns/iter (± 4) 717 ns/iter (± 5) 1.26
parse-ion 2510 ns/iter (± 26) 2263 ns/iter (± 5) 1.11
parse-group 7341 ns/iter (± 55) 7145 ns/iter (± 31) 1.03
parse-complex 19828 ns/iter (± 92) 18704 ns/iter (± 58) 1.06
parse-complex-fexpr 27149 ns/iter (± 181) 25839 ns/iter (± 81) 1.05

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.