Python extractor: overlay support

[d10c]

This PR adds overlay support to the Python extractor, but no overlay compilation (to be merged separately since it needs further testing, see this PR).

This PR also includes an initial pass at the discard predicates (see Overlay.qll), though these are ignored in full (non-overlay) evaluation; they probably still need to be tweaked, so I'm happy to move this commit to another PR and let this one be only about the extractor.

Roadmap:

• Update the dbscheme
• Implement path transformer support
• Read the overlay-changes JSON file
• Read/write base metadata (CODEQL_EXTRACTOR_<LANG>_OVERLAY_BASE_METADATA_{IN,OUT})

[d10c]

@tausbn I'm thinking this might be a good time to checkpoint this work and get it reviewed. In the last DCA run for full analysis on this PR (see above), overall analysis time is unaffected, though there are a few outstanding stage timing results that are probably noise.

[Copilot]

Pull Request Overview

This PR adds overlay support to the Python extractor by implementing infrastructure for incremental analysis through database overlays, without including overlay compilation functionality.

Key changes implemented:

• Database schema updates to support overlay metadata and change tracking
• Extractor modifications to handle overlay-specific file traversal and metadata management
• Path transformer support using updated environment variables

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
python/ql/lib/semmlecode.python.dbscheme	Adds databaseMetadata and overlayChangedFiles relations for overlay support
python/ql/lib/semmle/python/Overlay.qll	Implements discard predicates to filter out obsolete entities during overlay analysis
python/extractor/semmle/traverser.py	Modifies file traversal to only process changed files during overlay extraction
python/extractor/semmle/worker.py	Adds support for writing base metadata output required for overlay operations
python/extractor/semmle/path_rename.py	Updates path transformer to support new CODEQL_PATH_TRANSFORMER environment variable

[Copilot]

The variable name self.overlay_changes is inconsistent with the other instance variables which use snake_case (self.exclude_paths, self.recurse_files, etc.). Consider renaming to self.overlay_changed_paths for consistency.

[tausbn]

Overall I think this looks good. 👍

Do we have any tests for this? I feel like we might want to have a few CLI Integration tests to check that the overlay JSON files are being applied correctly. (The integration tests live here: https://github.com/github/codeql/tree/main/python/extractor/cli-integration-test)

Also, don't forget to update the extractor version here: https://github.com/github/codeql/blob/main/python/extractor/semmle/util.py#L13
(In this case, I think bumping it to 7.1.4 would be fine. We don't really have fixed rules for how to increase the version. The most important thing is that it changes so that we can tell from the log output what version of the extractor we're running.)

[tausbn]

I'm debating whether we should have some exception handling here (substituting the empty list of changed files in case something goes wrong). Currently, if something ends up being messed up in the JSON, then I believe the whole extraction will just fail.

I don't have strong feelings about it, though.

[d10c]

Thanks for the review! I also don't have strong opinions about whether file reading should fail loudly or warn and continue with a default (None, i.e. full extraction). I guess I'll go for the latter. And also insert a logger statement with the value of the environment variable, as is the convention elsewhere in the extractor.

[d10c]

Do we have any tests for this? I feel like we might want to have a few CLI Integration tests to check that the overlay JSON files are being applied correctly. (The integration tests live here: https://github.com/github/codeql/tree/main/python/extractor/cli-integration-test)

There are basic integration tests here but they depend on overlay compilation (not part of this commit), and also I'm still running into some issues on Windows (it appears that the path transformer is not working correctly there—currently debugging that). So maybe merging this should wait until I have that sorted.

Otherwise, do you have an idea for an integration test for this functionality that doesn't also exercise complete overlay evaluation?

[d10c]

I think I've figured out why path transformers weren't working on Windows and why built-in modules were being extracted (see latest commits). Now the integration test on the other PR passes.

The only remaining thing now is solving some tuple count regressions uncovered through DCA, but that can be done independently of this PR.

[jbj]

I've written a few comments after reading the discard predicates. I haven't reviewed the rest of this PR.

[jbj]

Suggested change

	overlay[discard_entity]
	privatepredicatediscardLocation(@location loc){
	/**
	*Locationsin Python TRAPfiles use named ids, so the overlay database will
	*reuselocationentitiesfrom the base. Therefore we should only discard a
	*locationifit'snotinusebythe overlay.
	*
	*Ifthe same element (with a named TRAPid)couldhaveadifferent location
	*inbaseandoverlay,thisdiscardingstrategywouldnotpreventthat element
	*from appearing to have two locations. However, the Python extractor does not
	*usenamedidsforentitiesthatcanchange location.
	*/
	overlay[discard_entity]
	privatepredicatediscardLocation(@location loc){

This is not an obvious predicate. I've suggested a comment here, but I don't even know if it's correct: is it impossible for the same @py_Module entity to have a different location in the base and the overlay?

Also, maybe there should be a comment to say that if we don't discard locations, probably nothing bad will happen. There will just be some unattached locations.

[d10c]

Python locations use *-ids. Which I guess means that the same @py_Module extracted in the overlay would have a different location than in the base extraction. So the base location should be discarded.

[d10c]

Superceded by PR #20337