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

Index files are not created if the indexed dictionary is not a root object #822

Open
noha opened this issue Sep 26, 2024 · 3 comments
Open
Labels
bug Something isn't working

Comments

@noha
Copy link
Contributor

noha commented Sep 26, 2024

This looks like a regression. We had that problem already. The file to store the index is only created if the indexed dictionary is a root object / a cluster object.

@noha noha added the bug Something isn't working label Sep 26, 2024
@MarcusDenker
Copy link
Contributor

MarcusDenker commented Sep 30, 2024

This has to do with the transaction not been set in the indexed dictionary:

The indexed dictionary wants to tag the value as root, see SoilIndexedDictionary>>#at:put:

at: key put: anObject
	| objectId |
	objectId := transaction makeRoot: anObject.

But we created the dict without a transaction normally and it is easy to forget.

In SoilPersistentDictionary, we guard for nil, see SoilPersistentDictionary>>#at:put:

at: key put: value
	| return |
	transaction ifNotNil: [:tr | tr makeRoot: value].
	return := dict at: key put: value.
	transaction ifNotNil: [:tr | tr markDirty: self].
	^return

But we do not do it in SoilPersistentDictionary>>#add:

This is not a good solution, as we never make the value a root if the transaction is not set.

We need to revisit now to deal with this and make sure SoilPersistentDictionary and SoilIndexedDictionary work the same way wrt to how to set the tansaction.

@MarcusDenker
Copy link
Contributor

For the SoilIndexedDictionary, the transaction is only set in #soilClusterRootIn:, thus only a SoilIndexedDictionary that is a root itself knows the transaction

@MarcusDenker
Copy link
Contributor

Setting the transaction is not enough, if we add the accessor, the following tests fails when trying to read from the second transaction, we get a SoilIndexNotFound error:

testIndexedDictionaryNotRoot
	| tx tx2 |
	tx := soil newTransaction.
	tx root: Dictionary new.
	tx root at: #indexed put: dict.
	"We need to set the transaction"
	dict transaction: tx.
	dict at: #foo2 put: #two.
	dict at: #foo put: #one.
	tx commit.
	"The following tests that everything works"
	tx2 := soil newTransaction.
	"and test first"
	self assert: ((tx2 root at: #indexed) at: #foo) equals: #one. 
	self assert:  ((tx2 root at: #indexed) at: #foo2) equals: #two

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

2 participants