apache / apache/ossie

Snowflake converter turns a query source with a leading comment into a fake table

Open
#376 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
2.1k
Forks
267
Avg merge
4d 20h
Merged PRs (30d)
24

Description

If a dataset's `source` is a SQL query that starts with a comment, the Snowflake converter doesn't notice it's a query. It splits the text on dots, uppercases the pieces, and emits them as a physical table. No error, no warning.

Here's the smallest example. This is valid Ossie; the spec says `source` can be a table name or a query.

```yaml
- name: orders
source: |
-- revenue source
SELECT amount FROM db.schema.orders
```

Running it through `convert_ossie_to_snowflake` gives:

```yaml
base_table:
database: "-- REVENUE SOURCE\nSELECT AMOUNT FROM DB"
schema: SCHEMA
table: ORDERS
```

The expected output is `base_table: {definition: }`, which is what Snowflake documents for query-backed tables.

The same thing happens with a `/* */` comment, with a query wrapped in parentheses, with `SELECT*FROM` (no space), and with `\r\n` after `SELECT` or `WITH`. Reproduced on `main` at 50457d3.

**Why**

`_parse_source` in `converters/snowflake/src/ossie_snowflake/converter.py` decides "query or table" with one check:

```python
upper.startswith(("SELECT ", "SELECT\n", "SELECT\t", "WITH ", "WITH\n", "WITH\t"))
```

Anything that doesn't match is assumed to be a table name. That path accepts any three dot-separated chunks without checking they're identifiers, so SQL text gets chopped up and emitted as `database.schema.table`.

**Where else**

The Honeydew converter has the same check and labels the query as a table. The NVIDIA converter has it too; its "must be a physical table" guard is bypassed by the comment, and it emits the same fake table. The Omni converter uses a word-boundary regex and raises a clear error instead, which is the right behavior when unsure.

**Proposed fix**

Two small changes, per converter:

1. Skip leading whitespace, `--` and `/* */` comments, and opening parentheses before looking for `SELECT` or `WITH`, and match the keyword as a whole word. Keep the query text exactly as written.
2. On the table path, require each of the three parts to be a real identifier (plain or double-quoted). If not, raise instead of uppercasing it.

This is independent of the structured `source` work in #109 / #173 / #338, which all keep string sources supported.

I have a patch for the Snowflake converter with tests (all existing tests pass, 18 added) and can follow up for Honeydew and NVIDIA. Happy to open the PR if this sounds right.

Contributor guide

Open the contributing guide

Research direction

Start in converters/snowflake/src/ossie_snowflake/converter.py at _parse_source, then run the converter's existing tests and review the reported Snowflake cases. Compare the same query-or-table check in the Honeydew and NVIDIA converters, and use the Omni converter's behavior as a reference. Done means commented, parenthesized, and tightly spaced queries remain query definitions while invalid table paths raise instead of emitting fake tables.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
data
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.