Skip to content

Add generic field for engine configuration #270

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.

Sign up for GitHub

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

Open
davidwzhao wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add generic field for engine configuration #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ac43df4
Add proto message for configuring AST size
davidwzhao Jul 23, 2026
33ca23c
Whoops forgot field name
davidwzhao Jul 23, 2026
58b0044
Add ASTSizeLimit to configure grammar
davidwzhao Jul 23, 2026
3ce8732
Add ASTSizeLimit equality and extend Configure equality in Julia SDK
davidwzhao Jul 23, 2026
047616a
Regenerate parsers, printers, and test snapshots
davidwzhao Jul 23, 2026
17548ea
Another direction - add a generic config values dict
davidwzhao Jul 24, 2026
3774923
Replace ASTSizeLimit with generic configuration_values dict in Configure
davidwzhao Jul 24, 2026
954b548
Update Configure equality in Julia SDK for configuration_values
davidwzhao Jul 24, 2026
434cd3d
Regenerate parsers, printers, and test snapshots
davidwzhao Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions meta/src/meta/codegen_templates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ class BuiltinTemplate:
"make_empty_bytes": BuiltinTemplate('b""'),
"dict_from_list": BuiltinTemplate("dict({0})"),
"string_map_from_pairs": BuiltinTemplate("dict({0})"),
"value_map_from_pairs": BuiltinTemplate("dict({0})"),
"dict_get": BuiltinTemplate("{0}.get({1})"),
"dict_to_pairs": BuiltinTemplate("sorted({0}.items())"),
"value_map_to_pairs": BuiltinTemplate("sorted({0}.items())"),
"has_proto_field": BuiltinTemplate("{0}.HasField({1})"),
"string_to_upper": BuiltinTemplate("{0}.upper()"),
"string_in_list": BuiltinTemplate("{0} in {1}"),
Expand Down Expand Up @@ -146,8 +148,10 @@ class BuiltinTemplate:
"make_empty_bytes": BuiltinTemplate("UInt8[]"),
"dict_from_list": BuiltinTemplate("Dict({0})"),
"string_map_from_pairs": BuiltinTemplate("Dict({0})"),
"value_map_from_pairs": BuiltinTemplate("Dict({0})"),
"dict_get": BuiltinTemplate("get({0}, {1}, nothing)"),
"dict_to_pairs": BuiltinTemplate("sort([(k, v) for (k, v) in {0}])"),
"value_map_to_pairs": BuiltinTemplate("sort([(k, v) for (k, v) in {0}])"),
"has_proto_field": BuiltinTemplate("_has_proto_field({0}, Symbol({1}))"),
"string_to_upper": BuiltinTemplate("uppercase({0})"),
"string_in_list": BuiltinTemplate("({0} in {1})"),
Expand Down Expand Up @@ -263,8 +267,10 @@ class BuiltinTemplate:
"make_empty_bytes": BuiltinTemplate("[]byte{}"),
"dict_from_list": BuiltinTemplate("dictFromList({0})"),
"string_map_from_pairs": BuiltinTemplate("stringMapFromPairs({0})"),
"value_map_from_pairs": BuiltinTemplate("valueMapFromPairs({0})"),
"dict_get": BuiltinTemplate("dictGetValue({0}, {1})"),
"dict_to_pairs": BuiltinTemplate("dictToPairs({0})"),
"value_map_to_pairs": BuiltinTemplate("valueMapToPairs({0})"),
"has_proto_field": BuiltinTemplate("hasProtoField({0}, {1})"),
"string_to_upper": BuiltinTemplate("strings.ToUpper({0})"),
"string_in_list": BuiltinTemplate("stringInList({0}, {1})"),
Expand Down
10 changes: 10 additions & 0 deletions meta/src/meta/grammar.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1702,9 +1702,15 @@ def construct_configure(config_dict: Sequence[Tuple[String, logic.Value]]) -> tr
maintenance_level = transactions.MaintenanceLevel.MAINTENANCE_LEVEL_OFF
ivm_config: transactions.IVMConfig = transactions.IVMConfig(level=maintenance_level)
semantics_version: int = _extract_value_int64(builtin.dict_get(config, "semantics_version"), 0)
config_values_pairs: List[Tuple[String, logic.Value]] = list[Tuple[String, logic.Value]]()
for pair in config_dict:
if pair[0] != "semantics_version" and pair[0] != "ivm.maintenance_level":
builtin.list_push(config_values_pairs, pair)
configuration_values: Dict[String, logic.Value] = builtin.value_map_from_pairs(config_values_pairs)
return transactions.Configure(
semantics_version=semantics_version,
ivm_config=ivm_config,
configuration_values=configuration_values,
)

def construct_export_csv_config(
Expand Down Expand Up @@ -1777,6 +1783,8 @@ def is_default_configure(cfg: transactions.Configure) -> bool:
return False
if cfg.ivm_config.level != transactions.MaintenanceLevel.MAINTENANCE_LEVEL_OFF:
return False
if builtin.length(builtin.value_map_to_pairs(cfg.configuration_values)) != 0:
return False
return True


Expand All @@ -1789,6 +1797,8 @@ def deconstruct_configure(msg: transactions.Configure) -> List[Tuple[String, log
elif msg.ivm_config.level == transactions.MaintenanceLevel.MAINTENANCE_LEVEL_OFF:
builtin.list_push(result, builtin.tuple("ivm.maintenance_level", _make_value_string("off")))
builtin.list_push(result, builtin.tuple("semantics_version", _make_value_int64(msg.semantics_version)))
for pair in builtin.value_map_to_pairs(msg.configuration_values):
builtin.list_push(result, pair)
return builtin.list_sort(result)


Expand Down
9 changes: 9 additions & 0 deletions meta/src/meta/target_builtins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,15 @@ def is_builtin(name: str) -> bool:
)
register_builtin("dict_get", [DictType(K, V), K], OptionType(V))
register_builtin("dict_to_pairs", [DictType(K, V)], ListType(TupleType([K, V])))
# Typed map helpers for map<string, Value> proto fields. Same semantics as
# dict_from_list / dict_to_pairs, but Go generates typed helpers (*pb.Value)
# instead of generic interface{}/string helpers.
register_builtin(
"value_map_from_pairs",
[SequenceType(TupleType([K, V]))],
DictType(K, V),
)
register_builtin("value_map_to_pairs", [DictType(K, V)], ListType(TupleType([K, V])))

# === Protobuf operations ===
register_builtin("has_proto_field", [T, STRING], BOOLEAN) # msg.HasField(field_name)
Expand Down
13 changes: 13 additions & 0 deletions meta/src/meta/templates/parser.go.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,19 @@ func dictFromList(pairs [][]interface{{}}) map[string]interface{{}} {{
return result
}}

// valueMapFromPairs builds map[string]*pb.Value from (key, *pb.Value) pair rows.
func valueMapFromPairs(pairs [][]interface{{}}) map[string]*pb.Value {{
out := make(map[string]*pb.Value)
for _, pair := range pairs {{
if len(pair) >= 2 {{
k, _ := pair[0].(string)
v, _ := pair[1].(*pb.Value)
out[k] = v
}}
}}
return out
}}

// stringMapFromPairs builds map[string]string from (prop key value) pair rows.
func stringMapFromPairs(pairs [][]interface{{}}) map[string]string {{
out := make(map[string]string)
Expand Down
17 changes: 17 additions & 0 deletions meta/src/meta/templates/pretty_printer.go.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,23 @@ func listSort(pairs [][]interface{{}}) [][]interface{{}} {{
return pairs
}}

// valueMapToPairs converts map[string]*pb.Value to sorted key/value rows for pretty printing.
func valueMapToPairs(m map[string]*pb.Value) [][]interface{{}} {{
if len(m) == 0 {{
return nil
}}
keys := make([]string, 0, len(m))
for k := range m {{
keys = append(keys, k)
}}
sort.Strings(keys)
out := make([][]interface{{}}, 0, len(keys))
for _, k := range keys {{
out = append(out, []interface{{}}{{k, m[k]}})
}}
return out
}}

// dictToPairs converts map[string]string to sorted key/value rows for pretty printing.
func dictToPairs(m map[string]string) [][]interface{{}} {{
if len(m) == 0 {{
Expand Down
13 changes: 12 additions & 1 deletion meta/src/meta/type_env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,18 @@ def _proto_type_to_target(self, proto_field: ProtoField) -> TargetType:
# Handle map fields
if proto_field.is_map:
key_type = _scalar_to_target(proto_field.map_key_type)
value_type = _scalar_to_target(proto_field.map_value_type)
map_value_type = proto_field.map_value_type
if map_value_type in _PRIMITIVE_TO_BASE_TYPE:
value_type: TargetType = BaseType(
_PRIMITIVE_TO_BASE_TYPE[map_value_type]
)
elif map_value_type in self.parser.messages:
message = self.parser.messages[map_value_type]
value_type = MessageType(message.module, map_value_type)
else:
value_type = _scalar_to_target(
map_value_type
) # raises for unknown types
return DictType(key_type, value_type)

# Get base type
Expand Down
4 changes: 4 additions & 0 deletions proto/relationalai/lqp/v1/transactions.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ message Transaction {
message Configure {
int64 semantics_version = 1;
IVMConfig ivm_config = 2;

// A generic configuration dictionary. The engine can choose how to interpret any entries
// in this config dict.
map<string, Value> configuration_values = 3;
Comment on lines +20 to +22

@comnik comnik Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have thought about this a bit more now and I sadly think the whole Configure approach was a misstep.

Anyways, I think the TL;DR is that what you're introducing here is fine, with one small clarification:

Suggested change
// A generic configuration dictionary. The engine can choose how to interpret any entries
// in this config dict.
map<string, Value> configuration_values = 3;
// A generic configuration dictionary. The engine can choose how to interpret any entries
// in this config dict. Absence of an entry must be interpreted following protobuf default
// value semantics.
map<string, Value> configuration_values = 3;

This lets us relax the following two requirements from the old design doc:

The client must specify all parameters explicitly in the protobuf (otherwise it would be up to the engine version to determine the behaviour).

When (pretty-)printing LQP, all; configuration values must be printed, even if they match the current default configuration. This ensures that an old snapshot test or old log message will still be interpreted correctly.

This was overeager and is only needed if the engine is allowed to flip the default behaviour without also bumping the LQP semantic version. By disallowing that, clients can safely omit anything where the protobuf default value is what they want.

@davidwzhao davidwzhao Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, why do absent entries need to follow protobuf default value semantics? The engine should be free to choose whatever default it wants, as long as it stays consistent within the same LQP version, right?

@comnik comnik Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess you're right. It doesn't have to be protobuf specifically, it's more like absence should default to the safe behaviour and defaults shouldn't change between LQP versions, yes 👍 So feature flags should be implemented as enable_xyz not disable_xyz.

}

message IVMConfig {
Expand Down
Loading
Loading