stefan-egli commented on PR #1526: URL: https://github.com/apache/jackrabbit-oak/pull/1526#issuecomment-2222991078
With the default set to disable compression, in theory this PR shouldn't do no harm and could be merged, as changing the default is a conscious decision that could be done based on the requirement of the system and which DocumentStore is used. Having said that, I think the current state of `testInterestingStrings` isn't ideal, as it lists a broken surrogate in a test string, but then doesn't test it. So I'd say either it uses that broken surrogate in a test, or then shouldn't list it at all. Except, I think now that this problem with broken surrogate was identified, we need to address it, so we probably should have it in the test indeed. What we might do is have the test use different DocumentStore fixtures and test on memory, mongo and rdb *with* the broken surrogate property and verify that the behavior with or without compression is the same. That could mean that for mongo the fact that compression doesn't support broken surrogates turns out as a non-issue as mongo already doesn't support it. It would just be good to have a test that verifies that. For RDB it could be different: it could mean that if you want to use compression for RDB you'd have to accept that broken surrogates are not supported (unless we find a fast way to support it in compression). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
