Make FSTCompiler.compile() to only return the FSTMetadata

[dungba88]

Description

Make FSTCompiler.compile() to only return the FSTMetadata. Depending on whether the DataOutput used implements FSTReader or not, the returned FST might be unreadable. Thus having this method only return the FSTMetadata means we will never return an unusable FST.

Now to create the FST there will be 2 ways:

1. If a FSTReader DataOutput is used, we can create the FST with:

var fstMetadata = fstCompiler.compile();
var fst = FST.fromFSTReader(fstMetadata, fstCompiler.getFSTReader());

2. If a non-FSTReader DataOutput is used, we need to first create the corresponding DataInput, then call:

var fstMetadata = fstCompiler.compile();
if (fstMetadata == null) {
    return null; // FST is empty
}

// Create the DataInput
var dataInput = ...

var fst = new FST<>(fstMetadata, dataInput);

Note that this PR depends on 2 others to be merged first:

• Allow FST builder to use different writer (#12543)  #12624
• Remove FST constructors with DataInput for metadata #12803

FST.fromFSTReader will also handle the case where fstMetadata is null, in which case it would return null. It is essentially a sugar syntactic utility to make the migration easier. So we will change this code:

var fst = fstCompiler.compile();

to this:

var fst = FST.fromFSTReader(fstCompiler.compile(), fstCompiler.getFSTReader());

instead of:

var fstMetadata = fstCompiler.compile();
if (fstMetadata == null) {
    return;
}
var fst = new FST<>(fstMetadata, fstCompiler.getFSTReader());

[dungba88]

I'll put the CHANGES.txt for #12624 together with this

[dungba88]

It's no longer possible for user to create a FST that uses NullFSTReader, so this Javadoc will no longer holds. If users are able to create FST, then it will always be readable.

[dungba88]

This fromFSTReader is there to avoid the boilerplate null-check that each consumer must now do. Open for method name suggestion.

[dungba88]

As we can't pass actual null to FSTReader, NullFSTReader will be used instead.

[dungba88]

One of the point I'm unsure about this PR is that there is now 2 (obscured) ways to construct the FST, one using the DataInput+FSTStore and one using FSTReader returned by the on-heap DataOutput. If users don't read the Javadoc, they might be confused on how to get the FST. Maybe there could be other way which feels more natural.

[github-actions]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the [email protected] list. Thank you for your contribution!

[mikemccand]

Thank you stale bot!

@dungba88 -- what is the status of this change?

I think it makes sense to have two FST compile+consume paths -- one on heap, that you can (efficiently) consume (read) right away without writing FST to stable storage, another that writes and then reads from stable storage.

[dungba88]

Thanks bot and @mikemccand, apparently I forgot about this PR!

I'll make another revision. The rebase after conflict seems to break the build.

[github-actions]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the [email protected] list. Thank you for your contribution!

[mikemccand]

This looks great @dungba88, and, incredibly, no additional conflicts despite getting stale.

I'll merge soon. Thanks!

[mikemccand]

This is technically an API break, but FSTCompiler is an experimental API and effectively an internal Lucene datastructure, so I think we can safely backport to 9.x without deprecation path.

[dungba88]

Thank you for merging @mikemccand !

[mikemccand]

Thank you for merging @mikemccand !

Thank you!