Bug: MAP export leaves the Arrow entries struct nullable, so no MAP column can be read through the Arrow API

[alvorithm]

I hit this reading query results through the Arrow API from a JVM application. Verified against lbug 0.19.0 and a relwithdebinfo build of 3abdb0653.

What happens

Any query returning a MAP column is unreadable through the Arrow result API. The consumer rejects the schema Ladybug produces:

IllegalArgumentException: Map data should be a non-nullable struct type

Everything else in the same table exports fine. lbug 0.19.0 with arrow-java 18.2.0, one row:

query	getArrowSchema	detail
RETURN n.id	ok	Schema<n.id: Utf8>
RETURN n.s (STRUCT)	ok	Schema<n.s: Struct<a: Utf8, b: Int(64, true)>>
RETURN n.l (STRING[])	ok	Schema<n.l: List<l: Utf8>>
RETURN n.d (DOUBLE[4])	ok	Schema<n.d: FixedSizeList(4)<l: FloatingPoint(DOUBLE)>>
RETURN n.m (MAP)	fails	IllegalArgumentException: Map data should be a non-nullable struct type
RETURN n.*	fails	same

Because RETURN n.* and RETURN n include every column, one MAP anywhere in a table's schema makes the whole Arrow read path unusable for that table.

Why

ArrowConverter::setArrowFormat, LogicalTypeID::MAP case, src/common/arrow/arrow_converter.cpp:256:

child.children[0]->name = "entries";
setArrowFormat(rootHolder, **child.children, ListType::getChildType(dataType),
    fallbackExtensionTypes);
child.children[0]->children[0]->flags &=
    ~ARROW_FLAG_NULLABLE; // Map's keys must be non-nullable

The key is cleared. The entries struct itself is not, and initializeChild sets child.flags = ARROW_FLAG_NULLABLE for every child by default (:73). The Arrow columnar spec requires the entries struct of a map to be non-nullable as well, so consumers reject the schema.

Read straight off the C data interface, no binding involved (verify_map_flags.c below):

  map column  format=+m   nullable=1
  entries     name=entries  format=+s   nullable=1   <-- Arrow requires 0
  entries.key name=KEY      format=u    nullable=0
  entries.val name=VALUE    format=u    nullable=1

  RESULT: VIOLATES the Arrow map layout (entries must be non-nullable)

Fix, built and verified

         child.children[0]->name = "entries";
         setArrowFormat(rootHolder, **child.children, ListType::getChildType(dataType),
             fallbackExtensionTypes);
+        child.children[0]->flags &=
+            ~ARROW_FLAG_NULLABLE; // Map's entries struct must be non-nullable
         child.children[0]->children[0]->flags &=
             ~ARROW_FLAG_NULLABLE; // Map's keys must be non-nullable

Same check against a relwithdebinfo build of 3abdb0653 with the two lines applied:

	before	after
map column	nullable=1	nullable=1	correct, the column may be null
entries	nullable=1	nullable=0	the fix
entries.key	nullable=0	nullable=0	already correct
	VIOLATES the Arrow map layout	conforms to the Arrow map layout

make test: 2642 of 2643 pass, the same single failure as an unpatched checkout of 3abdb0653 (dictionary_bug~orb383_relationship_projection_obfuscated.AnonymousParquetDeleteReload, whose Parquet fixtures are gitignored at .gitignore:95).

What I could not verify end to end is the Java round trip with the fix applied, because the released com.ladybugdb:lbug artifact statically links its own engine build, so a locally patched liblbug does not reach it. The flag is what the exception names, and the flag is now correct; whether arrow-java is then fully happy is worth one run on your side before merging.

One thing I noticed while reading the exported schema and did not chase: the entries children are named KEY and VALUE in upper case, where the Arrow convention is key and value. Nothing rejected on that, and the C data interface does not treat child names as normative, so I mention it only in case it matters to a stricter consumer.

Tests

test/api/arrow_test.cpp is the file to extend. A case exporting a MAP(STRING, STRING) column and asserting ARROW_FLAG_NULLABLE is clear on both entries and entries.key, plus a round trip through the C data interface preserving the entries. verify_map_flags.c below is that assertion in standalone form and translates directly.

Related

This is the mirror of a requirement callers hit on the way in: building a MapVector for createArrowTable fails the same way unless the entries struct is constructed non-nullable, and arrow-java's MapVector.getWriter() promotes it to a sparse union unless you are careful. A sentence about the invariant in the Arrow docs would help producers as much as this fix helps consumers.

Reproducers

cc -o verify_map_flags verify_map_flags.c -I$LBUG/src/include -I$LBUG/src/include/c_api \
   -L$LBUG/build/relwithdebinfo/src -llbug -Wl,-rpath,$LBUG/build/relwithdebinfo/src
./verify_map_flags     # exit 0 when the layout conforms, 2 when it does not

verify_map_flags.c, the C-level flag check

/* Direct C-API check of the MAP Arrow-export nullable flags. */
#include <stdio.h>
#include <string.h>
#include "c_api/lbug.h"

static void run(lbug_connection* c, const char* q) {
    lbug_query_result r;
    if (lbug_connection_query(c, q, &r) != LbugSuccess) {
        printf("  query failed: %s\n", lbug_query_result_get_error_message(&r));
    }
    lbug_query_result_destroy(&r);
}

int main(void) {
    lbug_database db; lbug_connection conn;
    lbug_system_config cfg = lbug_default_system_config();
    if (lbug_database_init("/tmp/ctest-map.lbug", cfg, &db) != LbugSuccess) { puts("db init failed"); return 1; }
    if (lbug_connection_init(&db, &conn) != LbugSuccess) { puts("conn init failed"); return 1; }

    run(&conn, "CREATE NODE TABLE T(id STRING, m MAP(STRING, STRING), PRIMARY KEY(id));");
    run(&conn, "CREATE (:T {id:'r1', m: map(['k'],['v'])});");

    lbug_query_result res;
    if (lbug_connection_query(&conn, "MATCH (n:T) RETURN n.m;", &res) != LbugSuccess) {
        printf("query failed: %s\n", lbug_query_result_get_error_message(&res));
        return 1;
    }
    struct ArrowSchema schema;
    if (lbug_query_result_get_arrow_schema(&res, &schema) != LbugSuccess) {
        puts("get_arrow_schema FAILED");
        printf("  lbug_get_last_error(): %s\n", lbug_get_last_error() ? lbug_get_last_error() : "(null)");
        return 1;
    }
    struct ArrowSchema* col = schema.children[0];              /* the MAP column   */
    struct ArrowSchema* entries = col->children[0];            /* its entries struct */
    struct ArrowSchema* key = entries->children[0];            /* entries.key      */
    printf("  map column  format=%-4s nullable=%d\n", col->format, !!(col->flags & ARROW_FLAG_NULLABLE));
    printf("  entries     name=%-8s format=%-4s nullable=%d   <-- Arrow requires 0\n",
           entries->name, entries->format, !!(entries->flags & ARROW_FLAG_NULLABLE));
    printf("  entries.key name=%-8s format=%-4s nullable=%d   <-- Arrow requires 0\n",
           key->name, key->format, !!(key->flags & ARROW_FLAG_NULLABLE));
    if (entries->n_children > 1) {
        struct ArrowSchema* val = entries->children[1];
        printf("  entries.val name=%-8s format=%-4s nullable=%d\n",
               val->name, val->format, !!(val->flags & ARROW_FLAG_NULLABLE));
    }
    int ok = !(entries->flags & ARROW_FLAG_NULLABLE) && !(key->flags & ARROW_FLAG_NULLABLE);
    printf("\n  RESULT: %s\n", ok ? "conforms to the Arrow map layout"
                                  : "VIOLATES the Arrow map layout (entries must be non-nullable)");
    if (schema.release) schema.release(&schema);
    lbug_query_result_destroy(&res);
    lbug_connection_destroy(&conn); lbug_database_destroy(&db);
    return ok ? 0 : 2;
}

MapExportProbe.java, the consumer-visible symptom

import com.ladybugdb.Connection;
import com.ladybugdb.Database;
import com.ladybugdb.QueryResult;
import com.ladybugdb.Version;

import org.apache.arrow.memory.BufferAllocator;
import org.apache.arrow.memory.RootAllocator;
import org.apache.arrow.vector.VectorSchemaRoot;

/** What does an Arrow consumer see when a query returns a MAP column? */
public class MapExportProbe {

    static void must(Connection c, String cypher) {
        try (QueryResult r = c.query(cypher)) {
            if (!r.isSuccess())
                throw new RuntimeException(cypher + " -> " + r.getErrorMessage());
        }
    }

    static void probe(Connection conn, BufferAllocator alloc, String label, String cypher) {
        System.out.printf("| `%s` | ", label);
        try (QueryResult r = conn.query(cypher)) {
            if (!r.isSuccess()) {
                System.out.println("**fails** | query: " + r.getErrorMessage().trim() + " |");
                return;
            }
            try (VectorSchemaRoot root = r.getArrowSchema(alloc)) {
                System.out.println("ok | " + root.getSchema().toString().replace('\n', ' ') + " |");
            } catch (Throwable t) {
                System.out.println("**fails** | " + t.getClass().getSimpleName() + ": "
                        + String.valueOf(t.getMessage()).replace('\n', ' ') + " |");
            }
        } catch (Throwable t) {
            System.out.println("**fails** | " + t.getClass().getSimpleName() + ": "
                    + String.valueOf(t.getMessage()).replace('\n', ' ') + " |");
        }
    }

    public static void main(String[] args) {
        System.out.println("lbug " + Version.getVersion() + ", arrow-java "
                + org.apache.arrow.vector.util.VectorBatchAppender.class.getPackage()
                        .getImplementationVersion());
        try (BufferAllocator alloc = new RootAllocator();
                Database db = new Database();
                Connection conn = new Connection(db)) {

            must(conn, "CREATE NODE TABLE T(id STRING, m MAP(STRING, STRING), "
                    + "s STRUCT(a STRING, b INT64), l STRING[], d DOUBLE[4], PRIMARY KEY(id));");
            must(conn, "CREATE (:T {id:'r1', m: map(['k'],['v']), "
                    + "s: {a:'x', b:1}, l: ['p','q'], d: [1.0,2.0,3.0,4.0]});");

            System.out.println("\n| query | getArrowSchema | detail |");
            System.out.println("|---|---|---|");
            probe(conn, alloc, "RETURN n.id", "MATCH (n:T) RETURN n.id;");
            probe(conn, alloc, "RETURN n.s (STRUCT)", "MATCH (n:T) RETURN n.s;");
            probe(conn, alloc, "RETURN n.l (STRING[])", "MATCH (n:T) RETURN n.l;");
            probe(conn, alloc, "RETURN n.d (DOUBLE[4])", "MATCH (n:T) RETURN n.d;");
            probe(conn, alloc, "RETURN n.m (MAP)", "MATCH (n:T) RETURN n.m;");
            probe(conn, alloc, "RETURN n.*", "MATCH (n:T) RETURN n.*;");
        }
    }
}

Classpath is com.ladybugdb:lbug:0.19.0 plus org.apache.arrow:arrow-memory-netty, arrow-c-data and arrow-vector at 18.2.0. Run with --sun-misc-unsafe-memory-access=allow --enable-native-access=ALL-UNNAMED --add-opens=java.base/java.nio=ALL-UNNAMED.

Notes

Verified against lbug 0.19.0 and a relwithdebinfo build of 3abdb0653. src/common/arrow/arrow_converter.cpp has had no commits since the 0.18.2 tag, where I first saw this.