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

Enable Graphql queries to support total when using Lucene queries #11159

Closed
wants to merge 9 commits into from

Conversation

hyzx86
Copy link
Contributor

@hyzx86 hyzx86 commented Feb 11, 2022

Change the query schema to the following: add 'hasTotal' :true

{
  "type":"contentItem/TableProducts",
 "hasTotal":true
}

image

#6939

@hyzx86 hyzx86 requested a review from Skrypt as a code owner February 11, 2022 06:22
@hyzx86
Copy link
Contributor Author

hyzx86 commented Feb 11, 2022

Regarding the "hasTotal":true option, I want it to be the default
I notice this PR is in the works, maybe it can be released with this feature?
#11052

@aminmohammadi05
Copy link

What if when there are multiple content types?

[{
  "type":"contentItem/TableProducts",
 "hasTotal":true
},
{
  "type":"contentItem/Type2",
 "hasTotal":true
}]

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 20, 2022

@aminmohammadi05 have you try graphql ?

Copy link
Member

@hishamco hishamco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a quick review, please check the format in all the places

@@ -149,7 +164,7 @@ private FieldType BuildSchemaBasedFieldType(LuceneQuery query, JToken querySchem
JsonConvert.DeserializeObject<Dictionary<string, object>>(parameters)
: new Dictionary<string, object>();

var result = await queryManager.ExecuteQueryAsync(iquery, queryParameters);
var result = (await queryManager.ExecuteQueryAsync(iquery, queryParameters)) as OrchardCore.Lucene.LuceneQueryResults;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can do the LuceneQueryResults casting in the return

@@ -185,14 +199,163 @@ private FieldType BuildContentTypeFieldType(ISchema schema, string contentType,
var queryParameters = parameters != null ?
JsonConvert.DeserializeObject<Dictionary<string, object>>(parameters)
: new Dictionary<string, object>();

var result = await queryManager.ExecuteQueryAsync(iquery, queryParameters);
var result = (await queryManager.ExecuteQueryAsync(iquery, queryParameters)) as OrchardCore.Lucene.LuceneQueryResults;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@aminmohammadi05
Copy link

@aminmohammadi05 have you try graphql ?
Yes but it does not shown in the graphql

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2022

@aminmohammadi05 have you try graphql ?
Yes but it does not shown in the graphql

cuz , you haven't input the schema field

@aminmohammadi05
Copy link

@aminmohammadi05 have you try graphql ?
Yes but it does not shown in the graphql

cuz , you haven't input the schema field

How to do that?

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2022

@aminmohammadi05 have you try graphql ?
Yes but it does not shown in the graphql

cuz , you haven't input the schema field

How to do that?

image

@aminmohammadi05
Copy link

aminmohammadi05 commented Sep 21, 2022

@aminmohammadi05 have you try graphql ?
Yes but it does not shown in the graphql

cuz , you haven't input the schema field

How to do that?

image
I mean when I put type Product in type field it shows just Product type field in Graphiql but I need it to show Blog type field also
image

image

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2022

image

@aminmohammadi05
Copy link

image

This is a Lucene search query which I need to provide parameters for it to search the parameters in all the selected types: Product, Blog

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2022

image

@aminmohammadi05
Copy link

image

I know, I want to set the schema field of the Lucene query in a way that the "searchAll" query shows fields of the both Product and Blog types

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2022

#11052

I noticed that this function was merged into the main branch yesterday

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2022

I know, I want to set the schema field of the Lucene query in a way that the "searchAll" query shows fields of the both Product and Blog types

Lucene query should not do what you said. It stores some types in a table containing many fields
If you want to query two types at the same time, you may need to perform secondary filtering on the client

@aminmohammadi05
Copy link

aminmohammadi05 commented Sep 21, 2022

I know, I want to set the schema field of the Lucene query in a way that the "searchAll" query shows fields of the both Product and Blog types

Lucene query should not do what you said. It stores some types in a table containing many fields If you want to query two types at the same time, you may need to perform secondary filtering on the client

No problem, how can I query two types at the same time. As you said I can handle separating the result based on its type.

@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 21, 2022

No problem, how can I query two types at the same time. As you said I can handle separating the result based on its type.

have you try object type?

https://docs.orchardcore.net/en/latest/docs/reference/modules/Queries/#graphql

image

@aminmohammadi05
Copy link

aminmohammadi05 commented Sep 21, 2022

No problem, how can I query two types at the same time. As you said I can handle separating the result based on its type.

have you try object type?

https://docs.orchardcore.net/en/latest/docs/reference/modules/Queries/#graphql

image

Yeah I saw that, I wonder how to query two types at the same time. As you see the document only shows one type: "BlogPost". Can you provide an example?

@hyzx86 hyzx86 closed this Sep 26, 2022
@hyzx86
Copy link
Contributor Author

hyzx86 commented Sep 26, 2022

Not ready , I'm trying to implement more flexible schema configuration, such as

{
items: {"type":"Array","ofType":"Blog"},
total: {"type":"Integer"},
}

@hyzx86 hyzx86 reopened this Sep 26, 2022
@aminmohammadi05
Copy link

Change the query schema to the following: add 'hasTotal' :true

{
  "type":"contentItem/TableProducts",
 "hasTotal":true
}

image

#6939

I tried this one to return total records but it did not work. What else should I do to get the result?

@aminmohammadi05
Copy link

image
this is the schema

@aminmohammadi05
Copy link

image
And the GraphQl output

@hyzx86
Copy link
Contributor Author

hyzx86 commented Oct 23, 2022

Looks like the code is not work ,Do you use my branch directly?

@aminmohammadi05
Copy link

No I downloaded it via Nuget package manager in Visual Studio 2022 3 months ago

@hyzx86
Copy link
Contributor Author

hyzx86 commented Oct 23, 2022

No I downloaded it via Nuget package manager in Visual Studio 2022 3 months ago

This feature only work on my branch...

@aminmohammadi05
Copy link

So how can I get total records without using you branch? cuz I am in the middle of development and feel it risky to change the Orchard

@hyzx86
Copy link
Contributor Author

hyzx86 commented Oct 23, 2022

So how can I get total records without using you branch? cuz I am in the middle of development and feel it risky to change the Orchard

You can directly use the WebAPI : api/queries/{name}

https://docs.orchardcore.net/en/latest/docs/reference/modules/Queries/#breaking-changes

@aminmohammadi05
Copy link

Thanks a lot.

@hyzx86
Copy link
Contributor Author

hyzx86 commented Mar 29, 2023

@hishamco

I thought, is it possible to change the UI of LuceneQuery to add a checkbox, which can togdle the total field

image

What do you think?

@hyzx86 hyzx86 requested review from hishamco and removed request for Skrypt March 29, 2023 07:05
Copy link
Contributor

github-actions bot commented Feb 8, 2024

This pull request has merge conflicts. Please resolve those before requesting a review.

@Piedone
Copy link
Member

Piedone commented Mar 13, 2024

I think this would be useful, but only if you implement parity with the Elasticsearch queries too. @Skrypt could you please chime in here?

@Skrypt Skrypt requested review from Skrypt and removed request for hishamco March 13, 2024 17:58
@sebastienros
Copy link
Member

Why not always have a count property and if the provider supports it return the value or -1 if it's not available. 0 would mean no items when the query supports it.

@hyzx86
Copy link
Contributor Author

hyzx86 commented Mar 14, 2024

Of course that would be best, but that would introduce a disruptive change.
Because the current query api returns a collection type

@hyzx86
Copy link
Contributor Author

hyzx86 commented Mar 14, 2024

In addition, we have discussed quantity statistics implemented on SQL Query
#6939

Copy link
Contributor

It seems that this pull request didn't really move for quite a while. Is this something you'd like to revisit any time soon or should we close? Please comment if you'd like to pick it up.

@github-actions github-actions bot added the stale label Jun 27, 2024
Copy link
Contributor

Closing this pull request because it has been stale for very long. If you think this is still relevant, feel free to reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants