Skip to content
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.

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

Add jsonl support #1919

Merged
merged 5 commits into from May 23, 2024
Merged

Add jsonl support #1919

merged 5 commits into from May 23, 2024

Conversation

kfigiela
Copy link
Contributor

@kfigiela kfigiela commented May 6, 2024

JSON support is great, but JSONL is very common in streaming cases (e.g. logs).

$ cat test.jsonl
{"a": 1}
{"a": 2}
{"a": 42}

$ alasql "select * from jsonl('./test') where a > 1"
[
  {
    "a": 2
  },
  {
    "a": 42
  }
]

@mathiasrw
Copy link
Member

mathiasrw commented May 7, 2024

Love it. Good and solid functionality to be added. Thank you.

Any chance you can add a test file that confirms its working (so we detect if we break that later)?

Is .jsonl the right name? What about .ndjson (new line delimited json) or I have even seen .jsons ?

src/84from.js Outdated Show resolved Hide resolved
src/84from.js Outdated Show resolved Hide resolved
@mathiasrw
Copy link
Member

@kfigiela ?

@kfigiela
Copy link
Contributor Author

@mathiasrw, thanks for reminder. I'll try to address those over the weekend and add tests.

As far as the extension/naming, according to a blog post these are "competing", but practically identical standards. We only parse, so as long as JSON.parse can handle it we should be good. I think, it makes sense to register them both, so either file extension works.

@paulrutter
Copy link
Contributor

https://duckdb.org/2023/03/03/json.html uses ndjson indeed, would be good to rename.

@kfigiela
Copy link
Contributor Author

On the other hand, Google BigQuery mentions JSONL everywhere. Nevertheless, we can have both aliases working and this is what I have implemented. Otherwise, I addressed the above comments and added some tests.

@mathiasrw
Copy link
Member

Just provide all the most used names (alasql.jsonl = alasql.ndjson = alsql.....)

@mathiasrw
Copy link
Member

Ahh - you already did :)

src/84from.js Show resolved Hide resolved
src/15utility.js Fixed Show fixed Hide fixed
@mathiasrw mathiasrw merged commit b9447ae into AlaSQL:develop May 23, 2024
10 checks passed
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.

None yet

3 participants