Skip to content

Add comprehensive LookML adapter fixes and kitchen sink tests#27

Merged
nicosuave merged 2 commits intomainfrom
nicosuave/looker-kitchen-sink-test
Jan 3, 2026
Merged

Add comprehensive LookML adapter fixes and kitchen sink tests#27
nicosuave merged 2 commits intomainfrom
nicosuave/looker-kitchen-sink-test

Conversation

@nicosuave
Copy link
Copy Markdown
Member

Implement 4 major LookML adapter fixes: (1) recursive dimension reference resolution for ${dimension_name} SQL expansions, (2) filter syntax conversion to SQL CASE WHEN, (3) derived measure support (type: number), and (4) multi-hop join detection with adjacency graph building. Add 86 integration tests with real DuckDB execution across 8-entity e-commerce model covering 36 distinct measure types and join patterns.

…ests

Implement 4 major fixes to sidemantic's LookML adapter: (1) recursive dimension reference resolution for ${dimension_name} SQL expansions, (2) proper LookML filter syntax conversion to SQL CASE WHEN expressions, (3) derived measure support (type: number) referencing other measures, and (4) multi-hop join detection with adjacency graph building. Add 86 comprehensive integration tests with real DuckDB execution across 8-entity e-commerce model covering 36 distinct measure types and join patterns.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3310f909e2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +447 to +451
# Numeric value
filters.append(f"{{model}}.{field} = {value}")
else:
# String value
filters.append(f"{{model}}.{field} = '{value}'")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve LookML comparison operators in filters

LookML filter values can encode comparison operators (e.g. ">1000", "<=100" as used in tests/fixtures/lookml/advanced_measures.lkml), but this conversion always emits equality, so those filters become sale_amount = '>1000' instead of sale_amount > 1000. That changes the meaning of filtered measures like large_sales_count and will typically return 0 or incorrect results for numeric columns. The conversion needs to parse operator-prefixed values rather than treating them as literal strings.

Useful? React with 👍 / 👎.

Parse operator-prefixed filter values (">1000", "<=100", ">=5", "<10", "!=0")
as proper SQL comparisons instead of string equality. Previously these were
incorrectly converted to `field = '>1000'` instead of `field > 1000`.
@nicosuave nicosuave merged commit 498eb5f into main Jan 3, 2026
10 checks passed
@nicosuave nicosuave deleted the nicosuave/looker-kitchen-sink-test branch January 3, 2026 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant