Uploaded image for project: 'Jackrabbit Oak'
  1. Jackrabbit Oak
  2. OAK-3509

Lucene suggestion results should have 1 row per suggestion with appropriate column names

Details

    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • None
    • 1.2.14, 1.3.11, 1.4
    • lucene
    • None

    Description

      Currently suggest query returns just one row with rep:suggest() column containing a string that needs to be parsed.
      It'd better if each suggestion is returned as individual row with column names such as suggestion, weight(???), etc.

      (cc teofili)

      Attachments

        Issue Links

          Activity

            +1 - Given we already have notion of row and column such data can be mapped to those constructs and not require user of API to parse them

            chetanm Chetan Mehrotra added a comment - +1 - Given we already have notion of row and column such data can be mapped to those constructs and not require user of API to parse them

            I am keen to agree on this that it'd be much easier to exploit results.
            Also we could include optional additional data provided by the underlying index (and I think it'd be good to switch the spellcheck support to be consumed the same way).
            If I recall correctly there were concerns around virtual / null paths, catholicon what do you think?

            teofili Tommaso Teofili added a comment - I am keen to agree on this that it'd be much easier to exploit results. Also we could include optional additional data provided by the underlying index (and I think it'd be good to switch the spellcheck support to be consumed the same way). If I recall correctly there were concerns around virtual / null paths, catholicon what do you think?
            catholicon Vikas Saurabh added a comment -

            The null path/virtual row discussion most probably won't affect this improvement. I'm a little busy at some other work. I'd try to prepare a patch around this as soon I can get some time.

            About virtual columns, I'm not sure if it's acceptable to rely on a particular index type - but then, as a count argument, rep:suggest/spellcheck already rely on index being used as lucene/solr based... so, we already have a precedent.

            catholicon Vikas Saurabh added a comment - The null path/virtual row discussion most probably won't affect this improvement. I'm a little busy at some other work. I'd try to prepare a patch around this as soon I can get some time. About virtual columns, I'm not sure if it's acceptable to rely on a particular index type - but then, as a count argument, rep:suggest/spellcheck already rely on index being used as lucene/solr based... so, we already have a precedent.
            catholicon Vikas Saurabh added a comment -

            teofili, I can put up a patch for this one. But, it'd convenient to change both suggestion and spellcheck result rows. Btw, I'm just planning to support 2 columns (which can probably be extended later as you hinted above) - rep:spellcheck and jcr:score (where I'm planning to put value of weight of each suggestion as jcr:score). Does that sound fine?

            Also, this is clearly a backward compatibility issue. I'm not sure how we deal with that. Do we just document it? What our take usually about backporting (to say 1.2 or 1.0) for such issues?

            catholicon Vikas Saurabh added a comment - teofili , I can put up a patch for this one. But, it'd convenient to change both suggestion and spellcheck result rows. Btw, I'm just planning to support 2 columns (which can probably be extended later as you hinted above) - rep:spellcheck and jcr:score (where I'm planning to put value of weight of each suggestion as jcr:score). Does that sound fine? Also, this is clearly a backward compatibility issue. I'm not sure how we deal with that. Do we just document it? What our take usually about backporting (to say 1.2 or 1.0) for such issues?

            I'm just planning to support 2 columns (which can probably be extended later as you hinted above) - rep:spellcheck and jcr:score (where I'm planning to put value of weight of each suggestion as jcr:score). Does that sound fine?

            +1

            Also, this is clearly a backward compatibility issue. I'm not sure how we deal with that. Do we just document it? What our take usually about backporting (to say 1.2 or 1.0) for such issues?

            yes that's a non backward compatible change, I think we should keep in trunk for 1.3+ releases and avoid backporting. We should document it on Oak Docs site e.g. "...since Oak 1.3.x suggest results are provided in separate rows ... "

            teofili Tommaso Teofili added a comment - I'm just planning to support 2 columns (which can probably be extended later as you hinted above) - rep:spellcheck and jcr:score (where I'm planning to put value of weight of each suggestion as jcr:score). Does that sound fine? +1 Also, this is clearly a backward compatibility issue. I'm not sure how we deal with that. Do we just document it? What our take usually about backporting (to say 1.2 or 1.0) for such issues? yes that's a non backward compatible change, I think we should keep in trunk for 1.3+ releases and avoid backporting. We should document it on Oak Docs site e.g. "...since Oak 1.3.x suggest results are provided in separate rows ... "
            catholicon Vikas Saurabh added a comment -

            Fixed in r1715716 and r1715717 (documentation)

            teofili, does committing lucene.md to trunk update the site automatically?

            catholicon Vikas Saurabh added a comment - Fixed in r1715716 and r1715717 (documentation) teofili , does committing lucene.md to trunk update the site automatically?
            amitjain Amit Jain added a comment -

            Bulk close for 1.3.11

            amitjain Amit Jain added a comment - Bulk close for 1.3.11
            catholicon Vikas Saurabh added a comment -

            Backported to 1.2 branch at r1738066.

            catholicon Vikas Saurabh added a comment - Backported to 1.2 branch at r1738066 .

            People

              catholicon Vikas Saurabh
              catholicon Vikas Saurabh
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: