Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions meta/src/meta/grammar.y
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,8 @@ export_iceberg_config
def _extract_value_int32(value: Optional[logic.Value], default: int) -> Int32:
if value is not None and builtin.has_proto_field(builtin.unwrap_option(value), 'int32_value'):
return builtin.unwrap_option(value).int32_value
if value is not None and builtin.has_proto_field(builtin.unwrap_option(value), 'int_value'):
return builtin.int64_to_int32(builtin.unwrap_option(value).int_value)

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.

Given that the S-expression syntax is meant for human readability, not necessarily writing convenience, I'd argue that the thing to do here for now is to assert that it's an int32_value.

Because if we allow promotion, we should do it generally, meaning anywhere that I write 1i32 today, I can just write 1 instead. But I'm not sure that convenience is worth having an implicit behaviour. The real problem here is the silent failure, not that we're strict.

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.

Makes sense. I will create another PR for that and close this one.

return builtin.int64_to_int32(default)


Expand Down
287 changes: 146 additions & 141 deletions sdks/go/src/parser.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion sdks/julia/LogicalQueryProtocol.jl/Project.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "LogicalQueryProtocol"
uuid = "a92373ee-6cc4-4662-ae66-0c99a03ebae1"
version = "0.5.5"
version = "0.5.6"

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.

Please keep the version bumps to separate PRs

authors = ["RelationalAI"]

[deps]
Expand Down
243 changes: 124 additions & 119 deletions sdks/julia/LogicalQueryProtocol.jl/src/parser.jl

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions sdks/julia/LogicalQueryProtocol.jl/test/parser_tests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,24 @@ end
)
end

@testitem "Parser - int32 config field accepts bare int" setup=[ParserSetup] begin
# A bare integer literal parses to `int_value`, an i32-suffixed one to
# `int32_value`. Both must land in the int32 `header_row` field identically;
# regression against silently falling back to the field default (1).
csv_fragment(header_row) = """
(fragment :f
(csv_data
(csv_locator (paths "s3://bucket/data.csv"))
(csv_config { :csv_header_row $header_row })
(columns (column "c" :c [INT]))
(asof "2025-01-01T00:00:00Z")))
"""
bare = Parser.parse_fragment(csv_fragment("-1"))
suffixed = Parser.parse_fragment(csv_fragment("-1i32"))
@test bare == suffixed
@test bare != Parser.parse_fragment(csv_fragment("1"))
end

@testitem "Parser - SYMBOL lexer regex" setup=[ParserSetup] begin
# Hyphen must be a literal character, not part of a range
lexer = Lexer("my-relation")
Expand Down
2 changes: 1 addition & 1 deletion sdks/python/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "lqp"
version = "0.5.5"
version = "0.5.6"
description = "Validate and translate Logical Query Protocol (LQP) S-expressions into Protobuf"
readme = "README.md"
authors = [
Expand Down
286 changes: 148 additions & 138 deletions sdks/python/src/lqp/gen/parser.py

Large diffs are not rendered by default.

21 changes: 21 additions & 0 deletions sdks/python/tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,27 @@ def test_parse_transaction_rejects_fragment():
parse_transaction(_SIMPLE_FRAGMENT)


def _csv_fragment(header_row: str) -> str:
return f"""
(fragment :f
(csv_data
(csv_locator (paths "s3://bucket/data.csv"))
(csv_config {{ :csv_header_row {header_row} }})
(columns (column "c" :c [INT]))
(asof "2025-01-01T00:00:00Z")))
"""


def test_int32_config_accepts_bare_int():
# A bare integer literal parses to `int_value`, an i32-suffixed one to
# `int32_value`. Both must land in the int32 `header_row` field identically;
# regression against silently falling back to the field default (1).
bare, _ = parse_fragment(_csv_fragment("-1"))
suffixed, _ = parse_fragment(_csv_fragment("-1i32"))
assert bare.declarations[0].data.csv_data.config.header_row == -1
assert bare == suffixed


class TestSymbolLexing:
"""Tests for SYMBOL token regex — hyphen must be literal, not a range."""

Expand Down
2 changes: 1 addition & 1 deletion sdks/python/uv.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading