make_qualified_base_name: qbn string view#419
Merged
villagestevers merged 2 commits intovillagesql:mainfrom May 5, 2026
Merged
make_qualified_base_name: qbn string view#419villagestevers merged 2 commits intovillagesql:mainfrom
villagestevers merged 2 commits intovillagesql:mainfrom
Conversation
The function takes const std::string& but its callers in util.cc:1289, 1332, 1420 pass `(const std::string ext_name, const char *)` — the const char* is silently wrapped in a temporary std::string. Switching to std::string_view lets all current callers pass through unchanged (std::string and const char* both convert implicitly) while removing those hidden allocations. Closes villagesql#416. Surfaced in villagesql#257 review thread; villagesql#408 deferred this.
pyshx
changed the title
pyshx
changed the title
This was referenced May 2, 2026
Per maintainer review: the std::string_view conversion in PR villagesql#419 left the body constructing two std::string temporaries plus operator+ intermediates — 2-4 allocations to do what should be 0-1. Switch to reserve + append/push_back for a single allocation when the result exceeds SSO.
villagestevers
approved these changes
May 5, 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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 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.
Description
make_qualified_base_name(villagesql/schema/systable/helpers.h:55) takesconst std::string&, but its callers inutil.cc:1289, 1332, 1420pass(const std::string ext_name, const char *)— theconst char *is silently wrapped in a temporarystd::stringto satisfy the signature. Switching tostd::string_viewlets all current callers pass through unchanged (std::stringandconst char *both convert implicitly) while removing those hidden allocations.Two other callers —
type_descriptor.h:156anddd/dd_routine.cc:89— pass(const std::string&, const std::string&)from accessor methods; those also remain unchanged.Related Issue
Closes #416.
How was this tested?
make -j12)./scripts/villint.sh --commit upstream/maincleanctest -L villagesql— 7/7 villagesql unit tests pass--do-suite=village --nounit-tests --parallel=auto— 552 tests pass, 8 skipped, 0 failures--suite=villagesql/examples/vsql-tvector— 6/6 pass--suite=villagesql/examples/vsql-complex— 7/7 passChecklist