feat: add NULL support for nullable fields and default values - #332
feat: add NULL support for nullable fields and default values#332MrLoh wants to merge 8 commits into
Conversation
0123113 to
78a1d22
Compare
Implement nullable field and default_value support matching Milvus Server behavior, fixing timeouts when inserting None into nullable scalar fields. Closes milvus-io#246 Co-authored-by: Cursor <cursoragent@cursor.com>
- Fix GetFieldDataCount using wrong accessor for BFloat16Vector fields - Fix AND/OR operator precedence in null-check expression parser - Fix parenthesized null checks like (field is null) not being recognized - Fix SliceFieldData test using wrong offset/count range - Add comprehensive Python integration tests for nullable edge cases Co-authored-by: Cursor <cursoragent@cursor.com>
Port IS NULL / IS NOT NULL support from upstream Plan.g4 into the ANTLR grammar and visitor, replacing the regex pre-processing layer. This removes ~200 lines of manual expression splitting, null-check regex matching, and operator-precedence logic from parser.cc. Add defensive validation in DecompactFieldData (valid_data true-count must match physical data count) and reject nullable vectors early in InsertTask::Process. Consolidate FillPlaceholderValues and FillDefaultValue into a single FillFieldValues function to reduce duplicated type-dispatch switches. Revert --update from conan install: it pulls binary packages built against newer GLIBC that are incompatible with the manylinux_2_28 CI container. Co-authored-by: Cursor <cursoragent@cursor.com>
ae92b3c to
0a1a4e9
Compare
Keep --update in conan install so stale recipe revisions in the persistent CI Conan cache get refreshed, but force bison/flex/libtool/openssl to build from source on Linux to avoid remote binaries compiled against GLIBC_2.29+. Install missing deps in ci/entrypoint.sh (openblas-devel for cblas.h, perl-IPC-Cmd/perl-Digest-SHA for openssl source build) until the base image is rebuilt from scripts/Dockerfile.manylinux.*. Co-authored-by: Cursor <cursoragent@cursor.com>
The patch was applied at line 332 of CMakeLists.txt, after add_subdirectory(knowhere) at line 254. By the time the patch modified the knowhere CMakeLists.txt on disk, CMake had already processed the unpatched version — so the ivfrbq_wrapper source expansion and cblas.h path fix never took effect, causing IndexIVFRaBitQWrapper linker errors. Move the patch application before any include() or add_subdirectory() calls from the submodule, and switch the knowhere patching from fragile git-apply (which silently fails in dirty working trees) to idempotent CMake file(READ/REPLACE/WRITE) operations. Co-authored-by: Cursor <cursoragent@cursor.com>
f75c418 to
7d1f136
Compare
Bump pymilvus from 2.4.4 to 2.5.4 in test requirements — the old version depended on milvus-lite<2.5.0, causing pip to downgrade the freshly built 2.5.2 wheel to 2.4.12 from PyPI so tests never ran against the CI-built code. Remove the legacy pymilvus.Milvus class usage (removed in pymilvus 2.5): drop get_milvus() from util_pymilvus.py and the dead connect, dis_connect, args, and milvus fixtures from conftest.py. Fix test_milvus_client_collection_self_creation_default: - DataType.Array → DataType.ARRAY (enum naming) - metric_type "cosine" → "COSINE" (case-sensitive in C++) - Add missing client arg and [0] to prepare_index_params() call - Correct check_items to match schema field names (id_string, embeddings) and enable_dynamic_field=False - Fix list_indexes assertion to match actual field name Co-authored-by: Cursor <cursoragent@cursor.com>
7d1f136 to
83d13c6
Compare
…scripts Add .devcontainer/ with an AlmaLinux 8 base image (multi-arch, auto-detects x86_64/aarch64) containing GCC 12, Conan, Rust, and test dependencies. Rewrite CONTRIBUTING.md with devcontainer setup, build/test commands, release wheel instructions, and code style conventions. Delete scripts/build_local.sh and scripts/test_local.sh (replaced by devcontainer). Improve ci/entrypoint.sh comments to document the stale CI image workarounds. Made-with: Cursor
96e298d to
a770ca0
Compare
|
@junjiejiangjjj any chance you could take a look at this. I could also separate this into 2 PRs: one that improves the local dev setup and fixes CI and one that adds the actual Null support feature, if that would be easier. |
|
@junjiejiangjjj what do I have to do to get any kind of traction on this? We are happy customers of milvus cloud, but having milvus lite for local research experiments is essential for us and the missing null support is a huge pain. While I'm not an expert in this codebase, I put serious effort into making this PR and I'd at least love some feedback. |
|
Hi @MrLoh, thanks for the contribution and sorry for the delay. The project is currently being rewritten in Python, so we’re unlikely to continue evolving the current codebase in this direction. Nullable support is planned in the rewritten version. Thanks again for the effort and follow-up. |
Delete tests/testcases/ and tests/customize/ (never run, no value for milvus-lite). Convert heavy imports in common_func.py to lazy imports. Remove unused `from utils.util_pymilvus import *` from all test files and dead legacy fixtures from conftest. Fix numpy.NaN removal, missing numpy alias, syntax error in search test, and argparse type bug. Add gitignore patterns for test DB artifacts. Update devcontainer with e2e test deps and fix CONTRIBUTING.md test instructions. Co-authored-by: Cursor <cursoragent@cursor.com>
Implement nullable field and
default_valuesupport matching Milvus Server behavior, fixing timeouts when insertingNoneinto nullable scalar fields. Also fixes the CI build pipeline and e2e test suite which had been broken, and adds a devcontainer for local development.Dev environment
The C++ build requires Conan 1.x, GCC 12, Rust, and specific system libraries (openblas, libatomic) that don't work with modern macOS toolchains. Previously there was no documented way to build locally — only CI could produce wheels. This made iterating on C++ changes impractical, which was a blocker for the nullable feature work.
Adds
.devcontainer/(AlmaLinux 8, multi-arch) with the full toolchain pre-installed so you can build and test inside the container on any host. RewritesCONTRIBUTING.mdwith setup instructions.Infrastructure fixes
The CI build and e2e tests had been broken for a while — the build image was stale and the test suite was pinned to an old
pymilvusversion that silently downgraded the freshly built wheel. These had to be fixed first to get a working feedback loop for the nullable feature.CI / build
--updatetoconan installwhile forcing source builds forbison/flex/libtool/opensslon Linux to avoid GLIBC_2.29+ incompatibilities in pre-built Conan binaries.ci/entrypoint.sh(openblas-devel,libatomic-static,perl-IPC-Cmd/perl-Digest-SHA) until the base image is rebuilt.milvus.patchapplication beforeadd_subdirectory(knowhere)in the rootCMakeLists.txtso the knowhere source-list patch takes effect.Test fixes
pymilvusfrom 2.4.4 to 2.5.4 — the old version pulledmilvus-lite<2.5.0from PyPI, so e2e tests never ran against the CI-built wheel.pymilvus.Milvuslegacy client usage that crashes on pymilvus 2.5+.test_milvus_client_collection_self_creation_default: correctDataType.Arrayenum casing, uppercasemetric_type, add missing args, and align assertions with actual schema field names.Pre-existing bug fixes
AddSchema/AddIndex(.c_str()truncates at embedded null bytes).BFloat16Vectorfield count readingfloat16_vector()instead ofbfloat16_vector().Nullable field support
Removes the explicit
"MilvusLite doesn't support nullable"block and implements the full nullable/default-value data path so thatnullable=Truefields anddefault_valuebehave identically to Milvus Server.Data path
PyMilvus sends nullable data in a compacted wire format where the payload only contains non-null values and a separate
valid_databitmask tracks nullability per row. Segcore expects uncompacted data where the payload length matchesnum_rows. The core change is a decompaction step in the insert path (DecompactFieldData) that expands compacted data, applies configured default values for omitted fields, and propagatesvalid_datathrough all downstream operations (index reordering, slicing, storage round-trips). Non-nullable fields withdefault_valueget filled withoutvalid_datato avoid falsely marking them as nullable downstream.Expression parsing
is null/is not nullfilter expressions are handled by extending the ANTLR grammar withISNULLandISNOTNULLlexer tokens and parser productions (ported from upstream Milvus Server'sPlan.g4), with corresponding visitor methods that buildNullExprprotobufs. All expression parsing stays in a single ANTLR pipeline — no regex pre-processing or manual operator-precedence logic.Closes #246