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

Filters for sub models don't generate the right subqueries #25

Open
RichardLindhout opened this issue May 14, 2020 · 30 comments
Open

Filters for sub models don't generate the right subqueries #25

RichardLindhout opened this issue May 14, 2020 · 30 comments
Assignees
Labels
bug Something isn't working

Comments

@RichardLindhout
Copy link
Member

RichardLindhout commented May 14, 2020

When I filter on

query{
  flowBlocks(filter:{
    where: {
      block:{
        title:{
          equalTo: "Wat wil je reinigen?"
        }
      }
    }
  }){
    id
    flow{
      title
    }
    block{
      id
      title
      organization{
        id
      }
    }
  }
}

It results in the following query:

SELECT * 
FROM   `flow_block` 
WHERE  `flow_block`.`organization_id` = 1 
       AND ( EXISTS(SELECT 1 
                    FROM   `block` 
                    WHERE  ( title = 'Wat wil je reinigen?' )) ); 

This results in all almost all flowblocks returning while it should only return flowblocks which contain a block with this title so the query should be:

SELECT * 
FROM   `flow_block` 
WHERE  `flow_block`.`organization_id` = 1 
       AND ( EXISTS(SELECT 1 
                    FROM   `block` 
                    WHERE  ( title = 'Wat wil je reinigen?' ) 
                           AND `flow_block`.`block_id` = `block`.`id`) ); 
@RichardLindhout RichardLindhout self-assigned this May 14, 2020
@RichardLindhout RichardLindhout added the bug Something isn't working label May 14, 2020
@RichardLindhout RichardLindhout changed the title Filters don't generate the right subqueries Filters for sub models don't generate the right subqueries May 14, 2020
@troian
Copy link

troian commented Jul 10, 2020

@RichardLindhout any luck to fix this?

here is an example of the issue in my case

SELECT * FROM "appointment" WHERE (((starts_at >= $1 AND starts_at < $2) OR (ends_at >= $3 AND ends_at < $4)) AND EXISTS(SELECT 1 FROM "appointment_participant" WHERE (account_id = $1)));
[2020-07-08 23:00:00 +0000 UTC 2020-07-08 23:20:00 +0000 UTC 2020-07-08 23:00:00 +0000 UTC 2020-07-08 23:20:00 +0000 UTC df95dc13-8673-40c0-ada4-203cf72e20ff]
{"level":"error","error":"models: failed to assign all query results to Appointment slice: bind failed to execute query: pq: operator does not exist: uuid = timestamp without time zone","time":"2020-07-10T03:50:27Z","message":"could not list appointments"}

account_id should pick argument 5, not 1

@troian
Copy link

troian commented Jul 10, 2020

I think issue is described here. cannot think about options to fix then

@RichardLindhout
Copy link
Member Author

Hey I'll try to invest some time in this soon, maybe today!

@RichardLindhout
Copy link
Member Author

What is the code what cause this it looks like this is another issue

@RichardLindhout
Copy link
Member Author

I ment what is the graphql filter you sent?

@troian
Copy link

troian commented Jul 10, 2020

{
    appointments(filter: {
        where : {
            startsAt: "2020-07-08T23:00:00Z",
            endsAt: "2020-07-08T23:20:00Z",
            appointmentParticipants : {
                account : {
                    id : {
                        equalTo: "df95dc13-8673-40c0-ada4-203cf72e20ff"
                    }
                }
            }
        }
    }) {
        id
        startsAt
        endsAt
        status
    }
}
	subQueryMods := AppointmentParticipantWhereToMods(m, !hasForeignKeyInRoot)
	if len(subQueryMods) > 0 {
		subQuery := models.AppointmentParticipants(append(subQueryMods, qm.Select("1"))...)
		queryMods = appendSubQuery(queryMods, subQuery.Query) // <-- issue happens here
	}

@RichardLindhout
Copy link
Member Author

RichardLindhout commented Jul 11, 2020

I can't reproduce the error you are having, but I think I know a way to fix this issue but I would like to make this library more polished and testable first. I don't have time in the next few weeks to work around this. Will surely fix in the future + new features but I have a few private projects I need to finish and work on before I can work on open source again.

Schermafbeelding 2020-07-11 om 14 17 16

@troian
Copy link

troian commented Jul 11, 2020

If you could drop me a few lines about fix idea you have I could try to make it work

@troian
Copy link

troian commented Jul 11, 2020

that quite dirty hack, but works

var (
	replaceQuestion = regexp.MustCompile(`(\$\d{1,2})`)
)

func appendSubQuery(queryMods []qm.QueryMod, q *queries.Query) []qm.QueryMod {
	qs, args := queries.BuildQuery(q)
	qsClean := strings.TrimSuffix(qs, ";")

	res := replaceQuestion.ReplaceAllStringFunc(qsClean, func(m string) string {
		parts := replaceQuestion.FindStringSubmatch(m)

		if len(parts) > 1 {
			return "?"
		}
		return ""
	})

	return append(queryMods, qm.Where(fmt.Sprintf("EXISTS(%v)", res), args...))
}

@troian
Copy link

troian commented Jul 11, 2020

i just think it probably worth to ask sqlboler authors to provide BuildQuery with option to not substitute params, not sure though it is a good idea. or use UseIndexPlaceholders set to false

@troian
Copy link

troian commented Jul 11, 2020

like this 🤣

func appendSubQuery(queryMods []qm.QueryMod, q *queries.Query) []qm.QueryMod {
	// all nasty job below is to tell query builder not to replace ? placeholders
	member := reflect.ValueOf(q).Elem().FieldByName("dialect")
	dialectPtr := (**drivers.Dialect)(unsafe.Pointer(member.UnsafeAddr()))
	dialect := **dialectPtr
	dialect.UseIndexPlaceholders = false
	*dialectPtr = &dialect

	qs, args := queries.BuildQuery(q)
	qsClean := strings.TrimSuffix(qs, ";")

	return append(queryMods, qm.Where(fmt.Sprintf("EXISTS(%v)", qsClean), args...))
}

@RichardLindhout
Copy link
Member Author

I think this issue is another problem too, even if the arguments worked there would still be issues where there are too much results. This looks more like a sqlboiler issue/enhancement.

Anyway I'm fixing the issue title and will try to look if I have the same issue with more filters

@RichardLindhout
Copy link
Member Author

RichardLindhout commented Jul 14, 2020

Fixed the issue described (not @troian 's case yet).

However it only works 2/3 level nested at the moment since we need to pass all other queryMods to the sub queries too so deeper filters still need a better bugfix for this so we can go endless deeply with filters

SELECT
   * 
FROM
   `block` 
WHERE
   `block`.`organization_id` = 1
   AND 
   (
      EXISTS
      (
         SELECT
            1 
         FROM
            `flow_block` 
         WHERE
            (
               EXISTS
               (
                  SELECT
                     1 
                  FROM
                     `block` 
                  WHERE
                     (
                        EXISTS
                        (
                           SELECT
                              1 
                           FROM
                              `block_choice` 
                           WHERE
                              (
                                 EXISTS
                                 (
                                    SELECT
                                       1 
                                    FROM
                                       `block` 
                                    WHERE
                                       (
                                          EXISTS
                                          (
                                             SELECT
                                                1 
                                             FROM
                                                `block_choice` 
                                             WHERE
                                                (
                                                   id = 11
                                                )
                                                AND 
                                                (
                                                   block_choice.block_id = block.id
                                                )
                                          )
                                       )
                                 )
                              )
                              AND 
                              (
                                 block_choice.block_id = block.id
                              )
                              
                        )
                     )
               )
            )
            AND 
            (
               flow_block.block_id = block.id
            )
      )
   )
;

Need to become

SELECT
   * 
FROM
   `block` 
WHERE
   `block`.`organization_id` = 1 
   AND 
   (
      EXISTS 
      (
         SELECT
            1 
         FROM
            `flow_block` 
         WHERE
            (
               EXISTS 
               (
                  SELECT
                     1 
                  FROM
                     `block` 
                  WHERE
                     (
                        EXISTS 
                        (
                           SELECT
                              1 
                           FROM
                              `block_choice` 
                           WHERE
                              (
                                 EXISTS 
                                 (
                                    SELECT
                                       1 
                                    FROM
                                       `block` 
                                    WHERE
                                       (
                                          EXISTS 
                                          (
                                             SELECT
                                                1 
                                             FROM
                                                `block_choice` 
                                             WHERE
                                                (
                                                   id = 11 
                                                )
                                                AND 
                                                (
                                                   block_choice.block_id = block.id 
                                                )
                                                AND 
                                                (
                                                   flow_block.block_id = block.id 
                                                )
                                          )
                                       )
                                 )
                              )
                              AND 
                              (
                                 block_choice.block_id = block.id 
                              )
                              AND 
                              (
                                 flow_block.block_id = block.id 
                              )
                        )
                     )
               )
            )
            AND 
            (
               flow_block.block_id = block.id 
            )
      )
   )
;

@RichardLindhout
Copy link
Member Author

RichardLindhout commented Jul 14, 2020

@troian Maybe something is wrong with the time filter.

I have the following query which I think is the same as yours relationship wise

query{
  blocks(filter:{
    where: {
      blockType:{
        equalTo: "CHOICE"
      }
      flowBlocks:{
        block:{
          id:{equalTo:"block-4"},
        }
      }
    }
  }){
    id
   title
    flowBlocks{
      id
      block{
      	id
        blockType
        blockChoices{
          id
          title
          block{
            id
          }
        }
      }
    }
  }
}
SELECT * FROM `block` WHERE `block`.`organization_id` = 1 AND (block_type = 'CHOICE' AND EXISTS(SELECT 1 FROM `flow_block` WHERE (block_id = 4) AND (flow_block.block_id = block.id)));

@RichardLindhout
Copy link
Member Author

Or maybe it's postgres related.. Hmm. Maybe we should ask Aaron.

@RichardLindhout
Copy link
Member Author

@troian try v2.1.4 and re-generate the filters

@troian
Copy link

troian commented Jul 14, 2020

The issue is definitely in there
Here is generated SQL query and the last placeholder is set tp $1 but should be $5

SELECT * FROM "appointment" WHERE (((starts_at >= $1 AND starts_at < $2) OR (ends_at >= $3 AND ends_at < $4)) AND EXISTS(SELECT 1 FROM "appointment_participant" WHERE (account_id = $1)));

@RichardLindhout
Copy link
Member Author

RichardLindhout commented Jul 14, 2020

How do get this sql, I only got ? inside of it. Or is this postgres related?

SELECT * FROM `block` WHERE `block`.`organization_id` = ? AND (block_type = ? AND EXISTS(SELECT 1 FROM `flow_block` WHERE (block_id = ?) AND (flow_block.block_id = block.id)));

@RichardLindhout
Copy link
Member Author

I think this is something sqlboiler should handle to be honest. It should see that subqueries are provided and ++ the indexes

@RichardLindhout
Copy link
Member Author

Ok forgot about this xD

func appendSubQuery(queryMods []qm.QueryMod, q *queries.Query) []qm.QueryMod {
	qs, args := queries.BuildQuery(q)
	qsClean := strings.TrimSuffix(qs, ";")
	return append(queryMods, qm.Where(fmt.Sprintf("EXISTS(%v)", qsClean), args...))
}

@troian
Copy link

troian commented Jul 14, 2020

Yah, issue happens when appendSubQuery in filter.go invokes BuildQuery from sqlboiler
Thats why rubbish below gets it to work

	member := reflect.ValueOf(q).Elem().FieldByName("dialect")
	dialectPtr := (**drivers.Dialect)(unsafe.Pointer(member.UnsafeAddr()))
	dialect := **dialectPtr
	dialect.UseIndexPlaceholders = false
	*dialectPtr = &dialect

@RichardLindhout
Copy link
Member Author

Ok, let's discuss this issue with Aaron :)

@RichardLindhout
Copy link
Member Author

Aaron said in Slack he has no time a.t.m. so I think we could maybe workaround this ourselves or use the reflect hack for the time being. Or do a pull request for sqlboiler.

I think my usage of the BuildQuery is not done before but the alternative is all raw queries which I want to prevent.

@troian
Copy link

troian commented Jul 20, 2020

Probably it worth to do it with reflect hack (or whatever option would fit). as this issue is quite a big hold

@RichardLindhout
Copy link
Member Author

I've proposed something to Aaron: volatiletech/sqlboiler#482 (comment)

We could probably export a more advanced query builder with some options

@RichardLindhout
Copy link
Member Author

Then we can disable the questionMarks without using reflect. But it would need a PR to sqlboiler

@RichardLindhout

This comment has been minimized.

@RichardLindhout
Copy link
Member Author

I'm interested though how this understands the arguments in the parent query understand that the arguments of the subquery. Can we have both query arguments in the parent query and the subquery etc . Before and after a sub query, we need some tests for this on sqlboiler too.

@troian
Copy link

troian commented Jul 25, 2020

@RichardLindhout i'll give it a check but atm it looks about right.
About substituting subqueries parameters I have exact same question. That was actually made me a surprise when trick with reflect did work.

@RichardLindhout
Copy link
Member Author

Hey @troian thanks for your workaround! I have released this under https://github.com/web-ridge/gqlgen-sqlboiler/releases/tag/v2.1.5

To enable it:

api.AddPlugin(gbgen.NewConvertPlugin(
			a,  
			b,  
			b, 
			gbgen.ConvertPluginConfig{
				UseReflectWorkaroundForSubModelFilteringInPostgresIssue25: true,
			},
		)),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants