Repository navigation
Setting trackerFileExtension to the instrument name fails with a SQL syntax error, or is silently ignored #311
Description
Summary
configure skips a trackerFileExtension that equals the instrument name (src/db_service.rs, update_instrument):
if let Some(ext) = &self.tracker_file_extension {
if ext != &self.name {
// extension defaults to instrument name
fields.push("tracker_file_extension=");
fields.push_bind_unseparated(ext);
}
}This causes two problems:
- SQL syntax error. When the extension is the only field being updated, the
is_empty()check passes (the extension isSome), but no field is added to theSETlist. The query becomesUPDATE instrument SET WHERE name = ? RETURNING *, and the mutation returns
error returned from database: (code: 1) near "WHERE": syntax error. - The extension can't be reset to the default. Once an extension has been set to something else, setting it back to the instrument name is silently dropped. If another field is updated in the same call, the mutation succeeds and returns the old extension unchanged.
A related inconsistency: creating an instrument with trackerFileExtension equal to its name stores it explicitly. The check only applies to updates. So two instances that behave identically can report null and "b01-1" for the same instrument.
Steps to reproduce (v1.1.0, also in v1.0.1)
git checkout v1.1.0
cargo build
./target/debug/numtracker serve --db repro.db --port 8123Run each mutation against http://localhost:8123/graphql, for example in graphiql:
1. Create an instrument without an extension. This works and returns trackerFileExtension: null.
mutation {
configure(instrument: "b01-1", config: {
directory: "/tmp/{instrument}/data/{year}/{visit}"
scan: "{subdirectory}/{instrument}-{scan_number}"
detector: "{subdirectory}/{instrument}-{detector}-{scan_number}"
}) { instrument trackerFileExtension }
}2. Set the extension to the instrument name. ❌ This returns the SQL syntax error.
mutation {
configure(instrument: "b01-1", config: { trackerFileExtension: "b01-1" }) { instrument trackerFileExtension }
}{"data":null,"errors":[{"message":"error returned from database: (code: 1) near \"WHERE\": syntax error","path":["configure"]}]}3. Set the extension to something else. This works and returns trackerFileExtension: "other".
mutation {
configure(instrument: "b01-1", config: { trackerFileExtension: "other" }) { instrument trackerFileExtension }
}4. Try to set it back to the instrument name, with another field. ❌ This succeeds, but the extension is still "other".
mutation {
configure(instrument: "b01-1", config: { trackerFileExtension: "b01-1", scanNumber: 5 }) { instrument trackerFileExtension }
}{"data":{"configure":{"instrument":"b01-1","trackerFileExtension":"other"}}}5. Create a different instrument with the extension equal to its name. This stores it explicitly (trackerFileExtension: "i22"), which isn't consistent with step 2.
mutation {
configure(instrument: "i22", config: {
directory: "/tmp/{instrument}/data/{year}/{visit}"
scan: "{subdirectory}/{instrument}-{scan_number}"
detector: "{subdirectory}/{instrument}-{detector}-{scan_number}"
trackerFileExtension: "i22"
}) { instrument trackerFileExtension }
}Expected
- Setting the extension to the instrument name updates it. In the database, that means setting
tracker_file_extensiontoNULL(the default) or to the name; either works, sinceNumTracker::for_instrumentfalls back to the instrument name withext.unwrap_or(bl). - An update that ends up with no fields to set returns the current configuration, as
is_empty()already does, instead of running an invalidUPDATE.
Suggested fix
In update_instrument, write NULL when the extension equals the name instead of skipping it:
if let Some(ext) = &self.tracker_file_extension {
fields.push("tracker_file_extension=");
// extension defaults to instrument name, so store NULL rather than duplicating it
fields.push_bind_unseparated((ext != &self.name).then_some(ext));
}This fixes all three problems: no empty SET, the extension can be reset, and the default is stored the same way every time. insert_new could normalise the same way for consistency.
How this was found
An integration test comparing the configurations query on prod and staging reported b01-1 as null on one and "b01-1" on the other. Trying to align them with the configure mutation hit the syntax error.
Activity