Add btree index filters to the C# sdk with codegen#1848
Add btree index filters to the C# sdk with codegen#1848
Conversation
|
What's the testing for this change? It looks like the test suite is failing - do existing tests need updating, and/or does it have new tests for the new behavior? |
|
This needs rebase now that Identity/Address representations have changed + snapshot regen. |
bfops
added
release-rc1
release-any
lcodes
force-pushed
the
jeremie/btree-index-bounds-cs
branch
from
067ffe6 to
f1eedf0
Compare
| let csharp_field_name_pascal = field_name.replace("r#", "").to_case(Case::Pascal); | ||
| let range = match field_type { | ||
| AlgebraicType::Product(pt) => | ||
| if pt.elements[0].name == Some("__address_bytes".into()) { ("Address.MinValue", "Address.MaxValue") } |
There was a problem hiding this comment.
We should reuse the is_address and other helpers so that we don't hardcode those names in more places.
| AlgebraicType::I8 => ("sbyte.MinValue", "sbyte.MaxValue"), | ||
| AlgebraicType::U8 => ("byte.MinValue", "byte.MaxValue"), | ||
| AlgebraicType::I16 => ("short.MinValue", "short.MaxValue"), | ||
| AlgebraicType::U16 => ("ushort.MinValue", "ushort.MaxValue"), | ||
| AlgebraicType::I32 => ("int.MinValue", "int.MaxValue"), | ||
| AlgebraicType::U32 => ("uint.MinValue", "uint.MaxValue"), | ||
| AlgebraicType::I64 => ("long.MinValue", "long.MaxValue"), | ||
| AlgebraicType::U64 => ("ulong.MinValue", "ulong.MaxValue"), | ||
| AlgebraicType::I128 => ("I128.MinValue", "I128.MaxValue"), | ||
| AlgebraicType::U128 => ("U128.MinValue", "U128.MaxValue"), | ||
| AlgebraicType::I256 => ("I256.MinValue", "I256.MaxValue"), | ||
| AlgebraicType::U256 => ("U256.MinValue", "U256.MaxValue"), | ||
| AlgebraicType::F32 => ("float.MinValue", "float.MaxValue"), | ||
| AlgebraicType::F64 => ("double.MinValue", "double.MaxValue"), |
There was a problem hiding this comment.
You should be able to use is_integer() || is_float() + scalar_or_string_name instead of repeating those.
| if pt.elements[0].name == Some("__address_bytes".into()) { ("Address.MinValue", "Address.MaxValue") } | ||
| else if pt.elements[0].name == Some("__identity_bytes".into()) { ("Identity.MinValue", "Identity.MaxValue") } | ||
| else { todo!() }, | ||
| AlgebraicType::String => ("\"\"", "\"\\uFFFF\\uFFFF\""), |
There was a problem hiding this comment.
I'm not sure this is correct. There isn't really a maximum string value, is there?
There was a problem hiding this comment.
This will probably require some logic revamp to account for types without a maximum.
| writeln!(output, "if ({0} != 0) return {0};", field.csharp_field_name_pascal); | ||
| }, | ||
| _ => { | ||
| writeln!(output, "if (a.{0} < b.{0}) return -1;", field.csharp_field_name_pascal); | ||
| writeln!(output, "if (a.{0} > b.{0}) return 1;", field.csharp_field_name_pascal); |
There was a problem hiding this comment.
All this shouldn't be necessary, you can just return the result of String.Compare as-is.
| match field.field_type { | ||
| AlgebraicType::Product(_) => { | ||
| writeln!(output, "var {0} = a.{0}.CompareTo(b.{0});", field.csharp_field_name_pascal); | ||
| writeln!(output, "if ({0} != 0) return {0};", field.csharp_field_name_pascal); |
There was a problem hiding this comment.
This shouldn't be necessary, you can just return result of CompareTo.
|
|
cloutiertyler
added
the
close-stale-pr-create-issue
6 participants
Description of Changes
Adds support for multi-column btree index filtering in the C# client
API and ABI breaking changes
No breaks.
Expected complexity level and risk
Most complexity is in the codegen, the runtime behavior is straightforward; a SortedSet<> per btree with a comparator matching its key columns, and Filter() methods querying that sorted set with min/max values derived from the table row type.
Testing