Skip to content

Maint/cleanup and merge lateral#6

Merged
teaguesterling merged 20 commits intomainfrom
maint/cleanup-and-merge-lateral
Dec 4, 2025
Merged

Maint/cleanup and merge lateral#6
teaguesterling merged 20 commits intomainfrom
maint/cleanup-and-merge-lateral

Conversation

@teaguesterling
Copy link
Owner

No description provided.

alexkwolfe and others added 4 commits November 24, 2025 22:25
Introduces *_each variants (git_tree_each, git_log_each, git_read_each,
git_parents_each, git_branches_each, git_tags_each) that support dynamic
parameters for LATERAL joins. Existing functions remain unchanged.

Architecture changes:
- Extract git functions from monolithic git_functions.cpp into separate files
- Add GitContextManager for centralized URI validation
- Implement per-thread repository caching (50-80% performance improvement)

Documentation:
- Add git_context_manager.md, advanced_examples.md, git_uris.md
- Add testing_strategy.md

Tests:
- 46 test cases with 736 assertions
- Comprehensive LATERAL join and thread-safety coverage
- Automated test fixture infrastructure

No breaking changes - all original signatures preserved.
…ions

Add LATERAL join support for git table functions
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
@teaguesterling
Copy link
Owner Author

@alexkwolfe I noticed that the Makefile had been changed leading to its default behavior no longer working. Was that intentional?

teaguesterling and others added 16 commits November 27, 2025 16:12
Add std::move() to enable implicit conversion from
unique_ptr<GitLogFunctionData> to unique_ptr<FunctionData>.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Create test/Makefile.inc with fixture extraction logic
- Add optional include from main Makefile (-include)
- Fixtures are automatically set up before test targets run

This keeps the main Makefile minimal while properly integrating
the test fixture setup from the contributor's PR.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Replace manual Unix-style path walking with libgit2's native discovery
mechanism for proper Windows and submodule support.

Changes:
- Use git_repository_discover() for cross-platform path resolution
- Use git_repository_workdir() to correctly handle submodules
- Fall back to git_repository_path() for bare repositories

This fixes Windows CI test failures where mingw-style paths weren't
being handled correctly, and also fixes submodule queries that were
returning empty results.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add cross-platform support note (Linux, macOS, Windows)
- Document git submodule query support
- Update test counts to 736 assertions across 46 test cases
- Clean up duplicated content in Current Status section
- Add LATERAL join support documentation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
The Windows builds were still running because the exclusion list had
"windows_amd64_minigw" instead of "windows_amd64_mingw" (extra 'i').

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Standardizes indentation (spaces to tabs) and reformats code
to match DuckDB style guide in preparation for enabling
code quality CI checks.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add test/fixtures/test_repo~/ to .gitignore
- Enable format check in CI (tidy disabled for now due to libgit2 deps)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add test/fixtures/test_repo~/ to .gitignore
- Enable format check in CI (tidy disabled for now due to libgit2 deps)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
Signed-off-by: Teague Sterling <teaguesterling@gmail.com>
@alexkwolfe
Copy link
Contributor

@teaguesterling Sorry! Just now seeing your question.

Dang it. That was unintentional. The lines adding dependencies to test_release_internal, test_debug_internal, and test_reldebug_internal were overriding targets from the upstream extension-ci-tools/makefiles/duckdb_extension.Makefile, which breaks the default test behavior. Looks like you restored the Makefile. Really sorry about that.

I was trying to ensure test fixtures were set up before running tests. Maybe wrapper targets like below would be better? Not sure the idiomatic way to handle this. Thoughts?

# Set up test fixtures before running tests
.PHONY: test_setup
test_setup:
    @echo "Setting up test fixtures..."
    @bash test/fixtures/setup_fixtures.sh setup

# Include the Makefile from extension-ci-tools
include extension-ci-tools/makefiles/duckdb_extension.Makefile

# Wrapper targets that add fixture setup WITHOUT modifying upstream behavior
.PHONY: test_with_fixtures
test_with_fixtures: test_setup test

.PHONY: test_release_with_fixtures
test_release_with_fixtures: test_setup test_release

.PHONY: test_debug_with_fixtures
test_debug_with_fixtures: test_setup test_debug

# Custom test target that ensures we don't use cached extensions
.PHONY: test_fresh
test_fresh: release test_setup
    @echo "Removing cached duck_tails extensions..."
    @rm -f ~/.duckdb/extensions/*/duck_tails.duckdb_extension
    @rm -f ~/.duckdb/extensions/*/*/duck_tails.duckdb_extension
    @echo "Running tests with fresh extension build..."
    ./build/release/test/unittest "$(PROJ_DIR)test/sql/*.test"

@teaguesterling
Copy link
Owner Author

No worries! I was able to get your changes integrated! Will be releasing an updated soon. Thank you for the PR (and will make sure to credit you 😀)

@teaguesterling teaguesterling merged commit 9b1b9fb into main Dec 4, 2025
36 checks passed
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