fix: sql parameterisation regressions#8709
Merged
chrisburr merged 10 commits intoJul 13, 2026
Merged
Conversation
Three defects introduced when FileManager.py was parameterised (PR DIRACGrid#8704): - _findFileIDs: the per-directory clause opened '( DirID=%s AND FileName IN (' but only closed the IN list, leaving the outer parenthesis unbalanced once clauses were OR-joined -> syntax error on every lookup (breaks getReplicas and any LFN->FileID resolution). - _getRepIDsForReplica: '(FileID,SEID) IN (' was filled with ','.join(['%s,%s']*n), yielding a flat scalar list 'IN (1,2,3,4)' against a 2-column row constructor -> MySQL error 1241. Use '(%s,%s)' groups so it renders 'IN ((1,2),(3,4))', and guard the empty-input case. - __getFileIDReplicas: 'args = fileIDs' aliased the caller's list and args.extend(statusIDs) mutated it, so the backfill in _getFileReplicas inserted status ids as bogus FileID keys -> KeyError in getReplicas. Copy the list before extending.
- getFileCounters: the 'Replicas w/o Files' query lost the closing parenthesis of its NOT IN (SELECT ...) subquery during parameterisation, so the counters RPC always failed with a syntax error. Restore the parenthesis. - _getFileAncestors / _getFileDescendents: 'args = fileIDs' aliased the caller's inputIDs list and args.extend(depths) mutated it, so the follow-up loop in _getFileRelatives iterated over the appended depth values and raised KeyError. Copy the list before extending.
Parameterising the query dropped the 'FROM FC_Files' clause, leaving 'SELECT FileName,... WHERE DirID=%s' which is invalid SQL. Every file lookup in FileManagerFlat (exists, addFile, removeFile, ...) therefore failed. Restore the FROM clause.
Three defects from the DatasetManager parameterisation:
- __getDatasetParameters: args was passed positionally to
self.db._query, whose 'args' parameter is keyword-only, raising
TypeError. This crashed every getDatasetParameters/Status/Files,
freezeDataset and releaseDataset call. Pass args=args.
- __freezeDataset: the multi-row INSERT concatenated value tuples with
no separator ('VALUES (a,b)(c,d)') -> syntax error. Join the groups
with commas and skip the INSERT when there are no files.
- __getDatasets: the wildcard form built 'dName' (with * -> % and
escaped _) but then bound the raw 'dsName', so LIKE searches never
matched and an empty basename bound one arg with no placeholder
(TypeError). Bind the value actually used and keep args in step with
the placeholders.
…rameter
Parameterisation changed 'WHERE DirID IN ( %s )' into
'WHERE DirID IN {dirIDString}', dropping the parentheses. Since
dirIDString is a bare id, comma list or SELECT sub-query, the result
('IN 42', 'IN 1,2,3', 'IN SELECT ...') is always a syntax error, so
every chmod/chown/chgrp/setDirectoryStatus on the directory tree failed.
Wrap the id string in parentheses again.
- lowerFields was a generator expression, so each 'field.lower() not in lowerFields' membership test consumed it; a second valid filter field was then wrongly rejected as an unknown field. Make it a list so it can be scanned repeatedly. - Each field's value alternatives were joined with OR but not parenthesised, so combining them with the other AND conditions gave 'A AND B OR C' precedence and returned wrong rows (and a wrong TotalRecords from the reused WHERE clause). Wrap each OR group in parentheses.
The parameterised UPDATE built its args list but never passed it to self._update, so the literal '%s' placeholders reached MySQL and the statement always failed with a syntax error.
Two stray print() calls left in during parameterisation dumped every profile-read query and its arguments to the service stdout on each call.
- getJobsAttributes is wrapped by @convertToReturnValue, which turns a plain 'return' into S_OK(...). Returning S_ERROR for an unknown attribute therefore produced S_OK(S_ERROR(...)), so callers saw OK=True and the error was swallowed. Raise SErrorException instead so the decorator emits a real S_ERROR. - insertNewJobIntoDB dropped the 'if not lfn: continue' guard and the lfn.strip() that the pre-parameterisation code (and the sibling setInputData) still apply, so empty-string InputData entries were inserted and jobs with no real input data were treated as input-data jobs. Restore the filter and strip.
- __writeBuckets: the loop reassigned sqlValues each iteration and the INSERT ran once afterwards, so only the last bucket of a multi-bucket record was written and the rest were silently dropped. Accumulate one placeholder group and its values per bucket and emit a real multi-row INSERT. - _bucketizeDataField: the emitted 'x % N' modulo collided with the driver's cmd % args formatting used by regenerateBuckets and __selectForCompactBuckets (both run with bound args), raising a formatting error mid-rebucket after the bucket table was already cleared. Double it to '%%' so it collapses back to a single '%'. - __selectForCompactBuckets: built its args list but never passed it to _query; pass args=args (also required for the '%%' collapse above). - getKeyValues: the correlation join 'keyTable.id = mainTable.key' for the key being listed was dropped, so its table cross-joined uncorrelated and every value was returned regardless of the conditions. Restore the join. - __selectIndividualForCompactBuckets: the 'LIMIT querySize' paging clause was lost, so the whole matching set was fetched in one query and the paging loop degenerated to a single pass. Restore the LIMIT.
chrisburr
requested review from
andresailer,
atsareg,
chaen and
fstagni
as code owners
July 13, 2026 09:04
sfayer
approved these changes
Jul 13, 2026
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.
BEGINRELEASENOTES
*DataManagementSystem
FIX: correct parameterised replica/file queries in FileManager (unbalanced IN parentheses, scalar list bound against the (FileID,SEID) row constructor, and argument-list aliasing that corrupted getReplicas)
FIX: repair getFileCounters and the file ancestor/descendent queries in FileManagerBase
FIX: restore the missing FROM clause in FileManagerFlat directory file listing
FIX: correct DatasetManager queries (keyword args to _query, comma-separated multi-row insert, and wildcard LIKE matching)
FIX: restore the IN-list parentheses when setting directory parameters in DirectoryTreeBase
*FrameworkSystem
FIX: correct getLogsContent field validation and OR-condition grouping in ProxyDB
FIX: remove stray debug prints from UserProfileDB.retrieveVarById
*StorageManagementSystem
FIX: pass the query arguments when updating stage request status in StorageManagementDB
*WorkloadManagementSystem
FIX: report unknown job attributes as an error (instead of a masked S_OK) and skip empty input-data LFNs in JobDB
*Accounting
FIX: correct AccountingDB bucket writing (multi-bucket records were silently dropped), rebucketing, key-value listing and bucket-compaction paging
ENDRELEASENOTES