Add custom query class that can be used to extend with query types not supported in this module

[juntezhang]

We are using later versions of ES, and there are new types of queries introduced not supported in this module. We need to be able to extend from a public class to create new query types. An example is the Text Expansion Query.

In order to create new queries in our codebase using this query builder, we can create custom extensions. However, a public custom query class is missing, making this not possible. This is the reason why I am proposing to add it to have this flexibility.

@sudo-suhas could you please check out this PR?

[sudo-suhas]

I am not sure this is all that useful. It does not have a way to set custom properties (such as model_id for text expansion query). If the custom query needs to be extended anyway, why not extend the Query class itself by importing it from core package?

[juntezhang]

The Query class cannot be extended because it is not a public class. We use this module as a dependency only and decorate it on our side. This is the minimum required to add extensions on our side.

On Sun, Aug 13, 2023, 11:13 AM Suhas Karanth @.***> wrote:

I am not sure this is all that useful. It does not have a way to set custom properties (such as model_id for text expansion query). If the custom query needs to be extended anyway, why not extend the Query class itself by importing it from core package?

— Reply to this email directly, view it on GitHub https://github.com/sudo-suhas/elastic-builder/pull/183#issuecomment-1676291617, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKDT2TIZ2XARCCCPSCS5V3XVCLCHANCNFSM6AAAAAA3KSCSSM . You are receiving this because you authored the thread.Message ID: @.***>

[sudo-suhas]

You can import the Query class with const { Query } = require('elastic-builder/src/core')

[juntezhang]

This won't work. You cannot extend the Query class because it is not public. You will get into circular dependency issues. Having a CustomQuery makes clearer that you can create a custom query class. This allows us to make it even compatible with custom queries created as plugins in either ES or OpenSearch. Alternatively making Query public would also work for us if you agree with this, then I will update this PR.

On Sun, Aug 13, 2023, 11:27 AM Suhas Karanth @.***> wrote:

You can import the Query class with const { Query } = require('elastic-builder/src/core')

— Reply to this email directly, view it on GitHub https://github.com/sudo-suhas/elastic-builder/pull/183#issuecomment-1676295806, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKDT2RCGQHNMYD55YL3L3DXVCMXHANCNFSM6AAAAAA3KSCSSM . You are receiving this because you authored the thread.Message ID: @.***>

[sudo-suhas]

We can export the Query class

[juntezhang]

alright, then I will update the PR next week to allow exporting the Query class.

On Sun, Aug 13, 2023, 12:55 PM Suhas Karanth @.***> wrote:

We can export the Query class

— Reply to this email directly, view it on GitHub https://github.com/sudo-suhas/elastic-builder/pull/183#issuecomment-1676318992, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKDT2SXAUGNLTAIQ5BF42LXVCXA3ANCNFSM6AAAAAA3KSCSSM . You are receiving this because you authored the thread.Message ID: @.***>

[juntezhang]

Author

I have tried to export Query but then we are getting a circular dependency issue.

export class NeuralSearchQuery extends Query {
^
TypeError: Class extends value undefined is not a constructor or null

I also tried to export MonoFieldQueryBase, but then getting the same:

export class NeuralSearchQuery extends MonoFieldQueryBase {
^
TypeError: Class extends value undefined is not a constructor or null

So the only way to be able to extend it without errors is using the CustomQuery class that I added in this PR. I hope you are able to approve it, thanks! @sudo-suhas

[juntezhang]

We found a way to make the Query class public on our side. We did it with this by declaring it:

declare module "elastic-builder/src/core/query" {
  declare class Query {
    constructor(queryType: string)
    boost(factor: number): this
    name(name: string): this
    getDSL(): object
    toJSON(): object
  }
  export default Query
}

Sharing this for others, and closing the PR.

[cimdalli]

@sudo-suhas just asking out of curiosity, is there any specific reason to not export Query class on index.js file? Since we are using typescript. in order to import specific sources, there should be a declaration of the imported module.

[juntezhang]

Yes, that would be the preferred solution if the Query class can be exported. I am not aware of reasons not to export this class.

On Thu, Aug 31, 2023, 11:43 AM Okan Cetin @.***> wrote:

@sudo-suhas https://github.com/sudo-suhas just asking out of curiosity, is there any specific reason to not export Query class on index.js file? Since we are using typescript. in order to import specific sources, there should be a declaration of the imported module.

— Reply to this email directly, view it on GitHub https://github.com/sudo-suhas/elastic-builder/pull/183#issuecomment-1700710845, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKDT2SLG5HPCQIKIB3UX3TXYBMCLANCNFSM6AAAAAA3KSCSSM . You are receiving this because you modified the open/close state.Message ID: @.***>

[sudo-suhas]

It wasn't exported earlier as I didn't think it would be useful. I have no objection to exporting it as such.

I had suggested that this PR could do the change for exporting it but @juntezhang reported it was causing a circular dependency issue. Although, I have no idea how exporting the class could lead to a circular dependency.

[juntezhang]

I think that was due to how I was trying to extend the class. I can create a new PR to make the Query class public.