You are viewing a plain text version of this content. The canonical link for it is here.
Posted to solr-dev@lucene.apache.org by "Kaktu Chakarabati (JIRA)" <ji...@apache.org> on 2009/05/07 09:55:33 UTC

[jira] Created: (SOLR-1149) Make QParserPlugin and related classes extendible

Make QParserPlugin and related classes extendible
-------------------------------------------------

                 Key: SOLR-1149
                 URL: https://issues.apache.org/jira/browse/SOLR-1149
             Project: Solr
          Issue Type: Improvement
          Components: search
            Reporter: Kaktu Chakarabati
             Fix For: 1.4


In a recent attempt to create a QParserPlugin which extends DisMaxQParser/FunctionQParser functionality, 
it became apparent that in the current state of these classes, it is not straight forward and in fact impossible to seriously build
upon the existing code. 
To this end, I've refactored some of the involved classes which enabled me to reuse existing logic to great results.
I thought I will share these changes and comment on their nature in the hope these will make sense to other solr developers/users, and
at the very least cultivate a fruitful discussion about this particular area of the solr codebase.

The relevant changes are as follows:

* Renamed DismaxQParser class to DisMaxQParser ( in accordance with the apparent naming convention, e.g DisMaxQParserPlugin )

* Moved DisMaxQParser to its own .java file, making it a public class rather than its previous package-private visibility. This makes
  it possible for users to build upon its logic, which is considerable, and to my mind is a good place to start alot of custom
  QParser implementations.

* Changed access modifiers for the QParser abstract base class to protected (were package-private). Again as above, it makes this
  object usable by user-defined classes that wish to define custom QParser classes. More generally, and on the philosophy-of-code 
  side of things, it seems misleading to define some class members as having the default access modifier (package-private) and then
  letting other package-scope derived classes use these while not explicitly allowing user-defined derived classes to make use of these members.

  In specific i'm thinking of how DisMaxQParser makes use of these members: **not because it is derived from QParser, but because it
  simply resides in the same namespace**

* Changed access modifier for the QueryParsing.StrParser inner class and its constructors to public. Again as in above, same issue
  of having same-package classes enjoy the benefit of being in the same namespace (FunctionQParser.parse() uses it like so), 
  while user defined classes cannot. Particulary in this case it is pretty bad since this class advertises itself as a collection of utilities
  for query parsing in general - great resource, should probably even live elsewhere (common.utils?)

* Changed Function.FunctionWeight inner class data member modifiers to protected (were default - package-private). This allowed me
  to inherit from FunctionQuery as well as make use of its original FunctionWeight inner class while overriding some of the latter's
  methods. This is in the same spirit of the changes above. Please also note this follows the common Query/Weight implementation pattern
  in the lucene codebase, see for example the BooleanQuery/BooleanWeight code.

All in all these are relatively minor changes which unlock a great deal of functionality to 3rd party developers, which i think is
ultimately a big part of what solr is all about - extendability. It is also perhaps a cue for a more serious refactoring of the
QParserPlugin hierarchy, although i will leave such bold exclamations to another occasion.

Attached is a patch file, having passed the usual coding-style/unit testing cycle.

-Chak

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-1149) Make QParserPlugin and related classes extendible

Posted by "Otis Gospodnetic (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-1149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12709104#action_12709104 ] 

Otis Gospodnetic commented on SOLR-1149:
----------------------------------------

It's set for release in 1.4, but subject to more review.
+1 from me.

> Make QParserPlugin and related classes extendible
> -------------------------------------------------
>
>                 Key: SOLR-1149
>                 URL: https://issues.apache.org/jira/browse/SOLR-1149
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>            Reporter: Kaktu Chakarabati
>             Fix For: 1.4
>
>         Attachments: SOLR-1149.patch
>
>
> In a recent attempt to create a QParserPlugin which extends DisMaxQParser/FunctionQParser functionality, 
> it became apparent that in the current state of these classes, it is not straight forward and in fact impossible to seriously build
> upon the existing code. 
> To this end, I've refactored some of the involved classes which enabled me to reuse existing logic to great results.
> I thought I will share these changes and comment on their nature in the hope these will make sense to other solr developers/users, and
> at the very least cultivate a fruitful discussion about this particular area of the solr codebase.
> The relevant changes are as follows:
> * Renamed DismaxQParser class to DisMaxQParser ( in accordance with the apparent naming convention, e.g DisMaxQParserPlugin )
> * Moved DisMaxQParser to its own .java file, making it a public class rather than its previous package-private visibility. This makes
>   it possible for users to build upon its logic, which is considerable, and to my mind is a good place to start alot of custom
>   QParser implementations.
> * Changed access modifiers for the QParser abstract base class to protected (were package-private). Again as above, it makes this
>   object usable by user-defined classes that wish to define custom QParser classes. More generally, and on the philosophy-of-code 
>   side of things, it seems misleading to define some class members as having the default access modifier (package-private) and then
>   letting other package-scope derived classes use these while not explicitly allowing user-defined derived classes to make use of these members.
>   In specific i'm thinking of how DisMaxQParser makes use of these members: **not because it is derived from QParser, but because it
>   simply resides in the same namespace**
> * Changed access modifier for the QueryParsing.StrParser inner class and its constructors to public. Again as in above, same issue
>   of having same-package classes enjoy the benefit of being in the same namespace (FunctionQParser.parse() uses it like so), 
>   while user defined classes cannot. Particulary in this case it is pretty bad since this class advertises itself as a collection of utilities
>   for query parsing in general - great resource, should probably even live elsewhere (common.utils?)
> * Changed Function.FunctionWeight inner class data member modifiers to protected (were default - package-private). This allowed me
>   to inherit from FunctionQuery as well as make use of its original FunctionWeight inner class while overriding some of the latter's
>   methods. This is in the same spirit of the changes above. Please also note this follows the common Query/Weight implementation pattern
>   in the lucene codebase, see for example the BooleanQuery/BooleanWeight code.
> All in all these are relatively minor changes which unlock a great deal of functionality to 3rd party developers, which i think is
> ultimately a big part of what solr is all about - extendability. It is also perhaps a cue for a more serious refactoring of the
> QParserPlugin hierarchy, although i will leave such bold exclamations to another occasion.
> Attached is a patch file, having passed the usual coding-style/unit testing cycle.
> -Chak

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SOLR-1149) Make QParserPlugin and related classes extendible

Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-1149?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shalin Shekhar Mangar resolved SOLR-1149.
-----------------------------------------

    Resolution: Fixed

Committed revision 777656.

Thanks Kaktu!

> Make QParserPlugin and related classes extendible
> -------------------------------------------------
>
>                 Key: SOLR-1149
>                 URL: https://issues.apache.org/jira/browse/SOLR-1149
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>            Reporter: Kaktu Chakarabati
>            Assignee: Shalin Shekhar Mangar
>             Fix For: 1.4
>
>         Attachments: SOLR-1149.patch, SOLR-1149.patch
>
>
> In a recent attempt to create a QParserPlugin which extends DisMaxQParser/FunctionQParser functionality, 
> it became apparent that in the current state of these classes, it is not straight forward and in fact impossible to seriously build
> upon the existing code. 
> To this end, I've refactored some of the involved classes which enabled me to reuse existing logic to great results.
> I thought I will share these changes and comment on their nature in the hope these will make sense to other solr developers/users, and
> at the very least cultivate a fruitful discussion about this particular area of the solr codebase.
> The relevant changes are as follows:
> * Renamed DismaxQParser class to DisMaxQParser ( in accordance with the apparent naming convention, e.g DisMaxQParserPlugin )
> * Moved DisMaxQParser to its own .java file, making it a public class rather than its previous package-private visibility. This makes
>   it possible for users to build upon its logic, which is considerable, and to my mind is a good place to start alot of custom
>   QParser implementations.
> * Changed access modifiers for the QParser abstract base class to protected (were package-private). Again as above, it makes this
>   object usable by user-defined classes that wish to define custom QParser classes. More generally, and on the philosophy-of-code 
>   side of things, it seems misleading to define some class members as having the default access modifier (package-private) and then
>   letting other package-scope derived classes use these while not explicitly allowing user-defined derived classes to make use of these members.
>   In specific i'm thinking of how DisMaxQParser makes use of these members: **not because it is derived from QParser, but because it
>   simply resides in the same namespace**
> * Changed access modifier for the QueryParsing.StrParser inner class and its constructors to public. Again as in above, same issue
>   of having same-package classes enjoy the benefit of being in the same namespace (FunctionQParser.parse() uses it like so), 
>   while user defined classes cannot. Particulary in this case it is pretty bad since this class advertises itself as a collection of utilities
>   for query parsing in general - great resource, should probably even live elsewhere (common.utils?)
> * Changed Function.FunctionWeight inner class data member modifiers to protected (were default - package-private). This allowed me
>   to inherit from FunctionQuery as well as make use of its original FunctionWeight inner class while overriding some of the latter's
>   methods. This is in the same spirit of the changes above. Please also note this follows the common Query/Weight implementation pattern
>   in the lucene codebase, see for example the BooleanQuery/BooleanWeight code.
> All in all these are relatively minor changes which unlock a great deal of functionality to 3rd party developers, which i think is
> ultimately a big part of what solr is all about - extendability. It is also perhaps a cue for a more serious refactoring of the
> QParserPlugin hierarchy, although i will leave such bold exclamations to another occasion.
> Attached is a patch file, having passed the usual coding-style/unit testing cycle.
> -Chak

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-1149) Make QParserPlugin and related classes extendible

Posted by "Kaktu Chakarabati (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-1149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12709048#action_12709048 ] 

Kaktu Chakarabati commented on SOLR-1149:
-----------------------------------------

Guys,
Any chance this will make it into the 1.4/1.5 roadmaps?
I think its a pretty minor change and can prove pretty useful ( for me and my company the alternative
would be to move to a roll-your-own solr war file mode which I'd really hate.. :) )

-Chak

> Make QParserPlugin and related classes extendible
> -------------------------------------------------
>
>                 Key: SOLR-1149
>                 URL: https://issues.apache.org/jira/browse/SOLR-1149
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>            Reporter: Kaktu Chakarabati
>             Fix For: 1.4
>
>         Attachments: SOLR-1149.patch
>
>
> In a recent attempt to create a QParserPlugin which extends DisMaxQParser/FunctionQParser functionality, 
> it became apparent that in the current state of these classes, it is not straight forward and in fact impossible to seriously build
> upon the existing code. 
> To this end, I've refactored some of the involved classes which enabled me to reuse existing logic to great results.
> I thought I will share these changes and comment on their nature in the hope these will make sense to other solr developers/users, and
> at the very least cultivate a fruitful discussion about this particular area of the solr codebase.
> The relevant changes are as follows:
> * Renamed DismaxQParser class to DisMaxQParser ( in accordance with the apparent naming convention, e.g DisMaxQParserPlugin )
> * Moved DisMaxQParser to its own .java file, making it a public class rather than its previous package-private visibility. This makes
>   it possible for users to build upon its logic, which is considerable, and to my mind is a good place to start alot of custom
>   QParser implementations.
> * Changed access modifiers for the QParser abstract base class to protected (were package-private). Again as above, it makes this
>   object usable by user-defined classes that wish to define custom QParser classes. More generally, and on the philosophy-of-code 
>   side of things, it seems misleading to define some class members as having the default access modifier (package-private) and then
>   letting other package-scope derived classes use these while not explicitly allowing user-defined derived classes to make use of these members.
>   In specific i'm thinking of how DisMaxQParser makes use of these members: **not because it is derived from QParser, but because it
>   simply resides in the same namespace**
> * Changed access modifier for the QueryParsing.StrParser inner class and its constructors to public. Again as in above, same issue
>   of having same-package classes enjoy the benefit of being in the same namespace (FunctionQParser.parse() uses it like so), 
>   while user defined classes cannot. Particulary in this case it is pretty bad since this class advertises itself as a collection of utilities
>   for query parsing in general - great resource, should probably even live elsewhere (common.utils?)
> * Changed Function.FunctionWeight inner class data member modifiers to protected (were default - package-private). This allowed me
>   to inherit from FunctionQuery as well as make use of its original FunctionWeight inner class while overriding some of the latter's
>   methods. This is in the same spirit of the changes above. Please also note this follows the common Query/Weight implementation pattern
>   in the lucene codebase, see for example the BooleanQuery/BooleanWeight code.
> All in all these are relatively minor changes which unlock a great deal of functionality to 3rd party developers, which i think is
> ultimately a big part of what solr is all about - extendability. It is also perhaps a cue for a more serious refactoring of the
> QParserPlugin hierarchy, although i will leave such bold exclamations to another occasion.
> Attached is a patch file, having passed the usual coding-style/unit testing cycle.
> -Chak

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (SOLR-1149) Make QParserPlugin and related classes extendible

Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-1149?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shalin Shekhar Mangar reassigned SOLR-1149:
-------------------------------------------

    Assignee: Shalin Shekhar Mangar

> Make QParserPlugin and related classes extendible
> -------------------------------------------------
>
>                 Key: SOLR-1149
>                 URL: https://issues.apache.org/jira/browse/SOLR-1149
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>            Reporter: Kaktu Chakarabati
>            Assignee: Shalin Shekhar Mangar
>             Fix For: 1.4
>
>         Attachments: SOLR-1149.patch
>
>
> In a recent attempt to create a QParserPlugin which extends DisMaxQParser/FunctionQParser functionality, 
> it became apparent that in the current state of these classes, it is not straight forward and in fact impossible to seriously build
> upon the existing code. 
> To this end, I've refactored some of the involved classes which enabled me to reuse existing logic to great results.
> I thought I will share these changes and comment on their nature in the hope these will make sense to other solr developers/users, and
> at the very least cultivate a fruitful discussion about this particular area of the solr codebase.
> The relevant changes are as follows:
> * Renamed DismaxQParser class to DisMaxQParser ( in accordance with the apparent naming convention, e.g DisMaxQParserPlugin )
> * Moved DisMaxQParser to its own .java file, making it a public class rather than its previous package-private visibility. This makes
>   it possible for users to build upon its logic, which is considerable, and to my mind is a good place to start alot of custom
>   QParser implementations.
> * Changed access modifiers for the QParser abstract base class to protected (were package-private). Again as above, it makes this
>   object usable by user-defined classes that wish to define custom QParser classes. More generally, and on the philosophy-of-code 
>   side of things, it seems misleading to define some class members as having the default access modifier (package-private) and then
>   letting other package-scope derived classes use these while not explicitly allowing user-defined derived classes to make use of these members.
>   In specific i'm thinking of how DisMaxQParser makes use of these members: **not because it is derived from QParser, but because it
>   simply resides in the same namespace**
> * Changed access modifier for the QueryParsing.StrParser inner class and its constructors to public. Again as in above, same issue
>   of having same-package classes enjoy the benefit of being in the same namespace (FunctionQParser.parse() uses it like so), 
>   while user defined classes cannot. Particulary in this case it is pretty bad since this class advertises itself as a collection of utilities
>   for query parsing in general - great resource, should probably even live elsewhere (common.utils?)
> * Changed Function.FunctionWeight inner class data member modifiers to protected (were default - package-private). This allowed me
>   to inherit from FunctionQuery as well as make use of its original FunctionWeight inner class while overriding some of the latter's
>   methods. This is in the same spirit of the changes above. Please also note this follows the common Query/Weight implementation pattern
>   in the lucene codebase, see for example the BooleanQuery/BooleanWeight code.
> All in all these are relatively minor changes which unlock a great deal of functionality to 3rd party developers, which i think is
> ultimately a big part of what solr is all about - extendability. It is also perhaps a cue for a more serious refactoring of the
> QParserPlugin hierarchy, although i will leave such bold exclamations to another occasion.
> Attached is a patch file, having passed the usual coding-style/unit testing cycle.
> -Chak

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-1149) Make QParserPlugin and related classes extendible

Posted by "Shalin Shekhar Mangar (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-1149?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Shalin Shekhar Mangar updated SOLR-1149:
----------------------------------------

    Attachment: SOLR-1149.patch

Added an experimental API note to all the changed classes.

I'll commit in a day or two.

> Make QParserPlugin and related classes extendible
> -------------------------------------------------
>
>                 Key: SOLR-1149
>                 URL: https://issues.apache.org/jira/browse/SOLR-1149
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>            Reporter: Kaktu Chakarabati
>            Assignee: Shalin Shekhar Mangar
>             Fix For: 1.4
>
>         Attachments: SOLR-1149.patch, SOLR-1149.patch
>
>
> In a recent attempt to create a QParserPlugin which extends DisMaxQParser/FunctionQParser functionality, 
> it became apparent that in the current state of these classes, it is not straight forward and in fact impossible to seriously build
> upon the existing code. 
> To this end, I've refactored some of the involved classes which enabled me to reuse existing logic to great results.
> I thought I will share these changes and comment on their nature in the hope these will make sense to other solr developers/users, and
> at the very least cultivate a fruitful discussion about this particular area of the solr codebase.
> The relevant changes are as follows:
> * Renamed DismaxQParser class to DisMaxQParser ( in accordance with the apparent naming convention, e.g DisMaxQParserPlugin )
> * Moved DisMaxQParser to its own .java file, making it a public class rather than its previous package-private visibility. This makes
>   it possible for users to build upon its logic, which is considerable, and to my mind is a good place to start alot of custom
>   QParser implementations.
> * Changed access modifiers for the QParser abstract base class to protected (were package-private). Again as above, it makes this
>   object usable by user-defined classes that wish to define custom QParser classes. More generally, and on the philosophy-of-code 
>   side of things, it seems misleading to define some class members as having the default access modifier (package-private) and then
>   letting other package-scope derived classes use these while not explicitly allowing user-defined derived classes to make use of these members.
>   In specific i'm thinking of how DisMaxQParser makes use of these members: **not because it is derived from QParser, but because it
>   simply resides in the same namespace**
> * Changed access modifier for the QueryParsing.StrParser inner class and its constructors to public. Again as in above, same issue
>   of having same-package classes enjoy the benefit of being in the same namespace (FunctionQParser.parse() uses it like so), 
>   while user defined classes cannot. Particulary in this case it is pretty bad since this class advertises itself as a collection of utilities
>   for query parsing in general - great resource, should probably even live elsewhere (common.utils?)
> * Changed Function.FunctionWeight inner class data member modifiers to protected (were default - package-private). This allowed me
>   to inherit from FunctionQuery as well as make use of its original FunctionWeight inner class while overriding some of the latter's
>   methods. This is in the same spirit of the changes above. Please also note this follows the common Query/Weight implementation pattern
>   in the lucene codebase, see for example the BooleanQuery/BooleanWeight code.
> All in all these are relatively minor changes which unlock a great deal of functionality to 3rd party developers, which i think is
> ultimately a big part of what solr is all about - extendability. It is also perhaps a cue for a more serious refactoring of the
> QParserPlugin hierarchy, although i will leave such bold exclamations to another occasion.
> Attached is a patch file, having passed the usual coding-style/unit testing cycle.
> -Chak

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-1149) Make QParserPlugin and related classes extendible

Posted by "Kaktu Chakarabati (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-1149?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kaktu Chakarabati updated SOLR-1149:
------------------------------------

    Attachment: SOLR-1149.patch

patch file implementing the mentioned changes.

> Make QParserPlugin and related classes extendible
> -------------------------------------------------
>
>                 Key: SOLR-1149
>                 URL: https://issues.apache.org/jira/browse/SOLR-1149
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>            Reporter: Kaktu Chakarabati
>             Fix For: 1.4
>
>         Attachments: SOLR-1149.patch
>
>
> In a recent attempt to create a QParserPlugin which extends DisMaxQParser/FunctionQParser functionality, 
> it became apparent that in the current state of these classes, it is not straight forward and in fact impossible to seriously build
> upon the existing code. 
> To this end, I've refactored some of the involved classes which enabled me to reuse existing logic to great results.
> I thought I will share these changes and comment on their nature in the hope these will make sense to other solr developers/users, and
> at the very least cultivate a fruitful discussion about this particular area of the solr codebase.
> The relevant changes are as follows:
> * Renamed DismaxQParser class to DisMaxQParser ( in accordance with the apparent naming convention, e.g DisMaxQParserPlugin )
> * Moved DisMaxQParser to its own .java file, making it a public class rather than its previous package-private visibility. This makes
>   it possible for users to build upon its logic, which is considerable, and to my mind is a good place to start alot of custom
>   QParser implementations.
> * Changed access modifiers for the QParser abstract base class to protected (were package-private). Again as above, it makes this
>   object usable by user-defined classes that wish to define custom QParser classes. More generally, and on the philosophy-of-code 
>   side of things, it seems misleading to define some class members as having the default access modifier (package-private) and then
>   letting other package-scope derived classes use these while not explicitly allowing user-defined derived classes to make use of these members.
>   In specific i'm thinking of how DisMaxQParser makes use of these members: **not because it is derived from QParser, but because it
>   simply resides in the same namespace**
> * Changed access modifier for the QueryParsing.StrParser inner class and its constructors to public. Again as in above, same issue
>   of having same-package classes enjoy the benefit of being in the same namespace (FunctionQParser.parse() uses it like so), 
>   while user defined classes cannot. Particulary in this case it is pretty bad since this class advertises itself as a collection of utilities
>   for query parsing in general - great resource, should probably even live elsewhere (common.utils?)
> * Changed Function.FunctionWeight inner class data member modifiers to protected (were default - package-private). This allowed me
>   to inherit from FunctionQuery as well as make use of its original FunctionWeight inner class while overriding some of the latter's
>   methods. This is in the same spirit of the changes above. Please also note this follows the common Query/Weight implementation pattern
>   in the lucene codebase, see for example the BooleanQuery/BooleanWeight code.
> All in all these are relatively minor changes which unlock a great deal of functionality to 3rd party developers, which i think is
> ultimately a big part of what solr is all about - extendability. It is also perhaps a cue for a more serious refactoring of the
> QParserPlugin hierarchy, although i will leave such bold exclamations to another occasion.
> Attached is a patch file, having passed the usual coding-style/unit testing cycle.
> -Chak

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.