Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle rt and plain indices the same #1249

Merged
merged 1 commit into from
Aug 11, 2023
Merged

handle rt and plain indices the same #1249

merged 1 commit into from
Aug 11, 2023

Conversation

akostadinov
Copy link
Contributor

@akostadinov akostadinov commented Aug 2, 2023

because of my oversight in #1222 all rt indices lost the sphinx_internal_class_name field and this is not correct. In either index type, when single table inheritance is used, then sphinx_internal_class_name should be present. Now also added a test for it - to make sure both index types are handled.

@akostadinov akostadinov marked this pull request as draft August 3, 2023 19:55
@@ -6,7 +6,7 @@
let(:indices) { [index_a, index_b] }
let(:index_a) { double 'Index A', :model => model_a, :type => 'plain',
:sources => [double(:fields => [field_a1, field_a2])] }
let(:index_b) { double 'Index B', :model => model_a, :type => 'rt',
let(:index_b) { double 'Index B', :model => model_b, :type => 'rt',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got me in trouble originally. index_a should use model_a and index_b should use model_b as far as I understand the intention of the test case.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As much as I can tell, yes, I think you're right! Thanks for this PR :)

@akostadinov akostadinov marked this pull request as ready for review August 10, 2023 14:28
@pat pat merged commit 5caa63f into pat:develop Aug 11, 2023
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