refactor: refactor json extension as built-in functionality#337
Merged
Conversation
shirly121
changed the title
| ] | ||
| statements = [] | ||
|
|
||
| for desc, stmt in statements: |
longbinlai
reviewed
May 13, 2026
| COPY person FROM "person.jsonl"; | ||
| ``` | ||
|
|
||
| > **Version Note:** In NeuG < 0.1.2, JSON support was provided via the [JSON Extension](../extensions/load_json) and required `INSTALL json; LOAD json;` before use. Since v0.1.2, this step is no longer needed. |
Collaborator
There was a problem hiding this comment.
Since version v0.1.2, we made json support as built-in functionality, so you donot need to install the json extension before using it. For NeuG version < 0.1.2, JSON support was provided via the JSON Extension and required INSTALL json; LOAD json; before use. 得差不多这样写一下
Collaborator
Author
There was a problem hiding this comment.
fixed in 0c7d432, include：
| # | Priority | Issue | File | Fix |
|---|---|---|---|---|
| 1 | Medium | compareVersion silently returns 0.0.0 on malformed version strings (e.g. 0.1.2-rc1) |
src/execution/execute/ops/admin/extension.cc |
Check sscanf return value; log WARNING and return false on parse failure (safe fallback: deprecation check is skipped) |
| 2 | Medium | doc/source/extensions/load_json.md has no mention that JSON is now built-in; link from load_data.md leads to outdated content |
doc/source/extensions/load_json.md |
Add a deprecation note at the top stating JSON is built-in since v0.1.2, with a link to the LOAD FROM reference |
| 3 | Medium | run_json_extension_suite name implies extension testing, but JSON is now built-in |
tools/python_bind/example/complex_test.py |
Rename to run_json_builtin_suite; update section title, log messages, and temp dir prefix; remove NEUG_RUN_EXTENSION_TESTS gate so JSON tests always run |
| 4 | Low | CMake comment does not explain why Arrow JSON is always enabled | CMakeLists.txt |
Add comment: JSON is now a core feature since v0.1.2, not an optional extension. |
| 5 | Medium | Dummy code: empty statements = [] with useless for-loop (zhanglei) |
tools/python_bind/example/complex_test.py |
Remove the empty statements list and its for-loop in run_json_builtin_suite |
| 6 | Medium | Version notes emphasize old behavior first; should lead with built-in (longbin) | doc/source/data_io/import_data.md, load_data.md, export_data.md, doc/source/extensions/load_json.md |
Rewrite version notes to state built-in functionality first, then mention legacy extension behavior |
No Changes Needed
| # | Issue | Reason |
|---|---|---|
| 1 | JSON tests not covered in core workflow | neug-test.yml already runs test_load.py and test_export.py without -k filter in Phase 3 — JSON tests are fully covered |
| 6 | Missing newline at end of file | Already fixed in the PR |
| 8 | @extension_test decorator removal impact |
The decorator only skips tests based on NEUG_RUN_EXTENSION_TESTS env var; removal does not affect test isolation or environment setup |
- Add sscanf return value check in compareVersion to handle malformed version strings gracefully - Remove dummy empty statements list in run_json_builtin_suite (zhanglei) - Rewrite version notes in docs to lead with built-in functionality instead of old extension behavior (longbin) - Rename run_json_extension_suite to run_json_builtin_suite and remove NEUG_RUN_EXTENSION_TESTS gate since JSON is now built-in - Add deprecation note to extensions/load_json.md - Improve CMake comment explaining why Arrow JSON is always enabled Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shirly121
force-pushed
the
remove_json_ext
branch
from
284c571 to
0c7d432
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What do these changes do?
Related issue number
Fixes #335