Skip to content

Maps allow integers as keys - #27

Open
stephenkeatley-eagleeye wants to merge 3 commits into
flix-tech:masterfrom
Eagle-Eye-Solutions:feature/AIR-33014
Open

Maps allow integers as keys#27
stephenkeatley-eagleeye wants to merge 3 commits into
flix-tech:masterfrom
Eagle-Eye-Solutions:feature/AIR-33014

Conversation

@stephenkeatley-eagleeye

Copy link
Copy Markdown

Added support so maps allow both strings and ints as keys.

Keys in PHP arrays will automatically cast to integers if all characters are integer digits. This change allows both strings and ints in the validation and then casts the key to a string when doing the actual encoding.

e.g.

{
  "mapContainer": {
    123: "value1",    // invalid before because it's an int, ok now
    "456": "value2",  // invalid before because array key gets converted to int, ok now
    "myKey": "value4" // ok before and now, stays as a string because not all int characters
  }
}

@stephenkeatley-eagleeye stephenkeatley-eagleeye changed the title AIR-33014: Maps allow integers as keys Maps allow integers as keys Apr 10, 2025
@tomsmith-eagleeye

Copy link
Copy Markdown

Merged master (8c040b4) in and added a test case.

Some extra detail on why this matters: without this change the library cannot read its own output. Given valid Avro bytes for the map {"123":1,"456":2}:

$schema = AvroSchema::parse('{"type":"map","values":"int"}');
$io = new AvroStringIO(hex2bin('040631323302063435360400'));
$read = (new AvroIODatumReader($schema))->read(new AvroIOBinaryDecoder($io));

var_export($read);                                       // array(123 => 1, 456 => 2)
var_export(AvroSchema::is_valid_datum($schema, $read));  // false

$out = new AvroStringIO();
(new AvroIODatumWriter($schema))->write($read, new AvroIOBinaryEncoder($out));
// AvroIOTypeException: The datum array (123 => 1, 456 => 2) is not an example of
// schema {"type":"map","values":{"type":"int"}}

The reader hands back int keys, because PHP casts digit-only array keys to int, and then the validator rejects them. A map read from one file cannot be written back out to another.

Both changed lines are needed, and they fail in different places. With only the schema.php change the datum validates, then the write fails further down the same call stack in AvroStringIO::write, which has its own is_string guard:

AvroIOException: write argument must be a string: (integer) 123

The new case in DatumIOTest::data_provider covers a map holding both key forms and asserts the exact bytes. It guards the two lines separately: drop the is_int check and it fails with AvroIOTypeException, drop the cast and it fails with the AvroIOException above. Full suite is green on 8.2 in the repo Dockerfile (480 tests, 1045 assertions, 0 failures); test_datum_round_trip also passes on 8.1 and 8.5.

For background on how we hit this: we serialise maps whose keys are user-supplied identifiers, so they arrive as a mix of "4323" and "A_String_Name". The numeric ones become ints on json_decode, and the whole map then failed to serialise.

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.

2 participants