Added Postgres Full Text Index together from Marten #110
Conversation
This file contains 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
…efinition Renamed setting DataConfig to DocumentCocument to align with PG naming and made it required Added option to specify FullTextIndex through ColumnExpression Added more tests
oskardudycz
force-pushed
the
full_text_index
branch
from
3303440 to
dfebada
Compare
…migration for Marten users Added more tests for FullTextIndexDefinition definition
Added common base class for index deltas detection tests
mysticmind
approved these changes
Dec 6, 2023
src/Weasel.Postgresql.Tests/Tables/Indexes/FullTextIndexDefinitionDeltasTests.cs
Outdated
Show resolved
Hide resolved
| set => regConfig = value ?? DefaultRegConfig; | ||
| } | ||
|
|
||
| public string DocumentConfig { get; set; } |
There was a problem hiding this comment.
I do understand, you are initializing it in ctor, but still, for the setter, wouldn't you check the value for null and set it to DataDocumentConfig?
There was a problem hiding this comment.
If we're using nullable reference types, the compiler will tell us we should not provide a null value there. If a user tries to cheat and forcibly put null, it's their mistake.
I'd like to get rid of DataDocumentConfig as it was my mistake to name it like that. Postgres names it document and if we put it into Weasel, then I think that we should keep naming generic.
I left DataDocumentConfig to not break people code when they upgrade Marten.
Thoughts?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Moved the Postgres Full Text Index code from Marten.
Adjusted the naming to the Weasel and Postgres conventions:
FullTextIndextoFullTextIndexDefinitionDataConfigtoDocumentConfigto align with PG naming and make it generic, as someone may use just regular column. KeptDataConfigas obsolete, to make easier transition for Marten users.Moved both the definition and delta detection tests. Added base class for index tests, as next to be moved will be NGram index.