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

Review formatting of boolean strings (in ToString() methods and similar) #921

Closed
paulirwin opened this issue Feb 25, 2024 · 1 comment
Closed
Assignees
Labels
is:enhancement New feature or request is:task A chore to be done pri:normal

Comments

@paulirwin
Copy link
Contributor

paulirwin commented Feb 25, 2024

Context: See PR #914

Any ideas what the string returned here is supposed to represent?

Java: spanNear([f:lucene, f:net, f:solr], 0, true)
.NET: SpanNear([f:lucene, f:net, f:solr], 0, True)

This looks like something that may be copied and pasted somewhere to me, but it may just be a textual description. If the former, we should probably at least strive to match the case of the boolean. I am guessing the name is the correct casing for .NET, though.

Of course, we are probably using upper case for all booleans converted to string in .NET, because that is what .NET does by default. So, whatever the fix, it should be applied consistently in all queries.

Originally posted by @NightOwl888 in #914 (comment)

@NightOwl888 NightOwl888 added up-for-grabs This issue is open to be worked on by anyone is:enhancement New feature or request pri:normal is:task A chore to be done labels Mar 10, 2024
@NightOwl888 NightOwl888 changed the title Review formatting of boolean strings Review formatting of boolean strings (in ToString() methods and similar) Mar 10, 2024
@paulirwin paulirwin added this to the 4.8.0-beta00018 milestone Oct 28, 2024
@paulirwin paulirwin self-assigned this Jan 5, 2025
@paulirwin paulirwin removed the up-for-grabs This issue is open to be worked on by anyone label Jan 5, 2025
@paulirwin
Copy link
Contributor Author

I reviewed all use of ToString overrides, and determined that nothing needs to be done here. If anyone else disagrees or finds anything actionable, let me know.

Below are the overrides that use raw booleans in the output string. In almost all cases, these simply seem to be for diagnostic/debugging output, and not for anything where the value is expected to be parsed or otherwise converted (in whole or in part) back to boolean values. "debug" below means that it is intended as debug output only. "not used" means it does not have any references to it in Lucene.NET code.

Types that use bool values directly in ToString() output:

  • LimitTokenCountAnalyzer - debug, not used
  • MemoryPostingsFormat - debug, not used
  • ProximityQueryNode - debug; interface denotes it's "for printing", not used
  • RandomSimilarityProvider - debug, not used
  • DocumentsWriterPerThread - debug, not used
  • LiveIndexWriterConfig - debug, used by IndexWriterConfig, but that usage is only used in a test. Used by IndexWriter just to write to infoStream.
  • LogMergePolicy - debug, not used
  • ReadersAndUpdates - debug, not used
  • FieldValueFilter - debug, not used
  • (potentially?) SortField - if m_missingValue is a boolean - used in various places, but nothing seems to indicate it would be affected as where it is used is i.e. looking for specific values
  • IOContext - debug, not used
  • MergeInfo - debug, not used
  • AttributeImpl - debug, but also LUCENENET specific, so no Java compat concerns
  • AttributeSource via ReflectAsString - only used in tests, ToString not used. ToString is debug output, ReflectAsString generally seems to be debug output and not intended as a serious form of serialization (for example, some strings would cause unparseable output)

Then, there are the Query overrides where it has a string field parameter, like the one that triggered the creation of this issue originally. I reviewed all usages of Query.ToString(string field) as well as Query.ToString() (which calls the former with "" as the parameter), and these also are only used for debug output and in assertion messages. It does not seem like it is intended to generate a parseable query string, and if it were intended to do so it seems like there would be several issues with this in the derived classes, like not properly escaping string values. So I think these can be safely ignored.

Query overrides of ToString(string field) containing booleans:

  • PayloadNearQuery
  • SpanNearQuery

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:enhancement New feature or request is:task A chore to be done pri:normal
Projects
None yet
Development

No branches or pull requests

2 participants