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

Postgres Eager loading SQL syntax error. #1589

Closed
vjames19 opened this Issue Apr 3, 2014 · 16 comments

Comments

8 participants
@vjames19

vjames19 commented Apr 3, 2014

When I execute db.GoesVariable.findAll({include: [db.GoesMap]});

This is what I get:

Executing (default): SELECT GoesVariables.*, GoesMaps.variableName AS GoesMaps.variableName, GoesMaps.imagePath AS GoesMaps.imagePath, GoesMaps.dataDate AS GoesMaps.dataDate, GoesMaps.createdAt AS GoesMaps.createdAt, GoesMaps.updatedAt AS GoesMaps.updatedAt FROM GoesVariables LEFT OUTER JOIN GoesMaps AS GoesMaps ON GoesVariables.variableName = GoesMaps.variableName;
{ [error: syntax error at or near "."]
  name: 'error',
  length: 83,
  severity: 'ERROR',
  code: '42601',
  detail: undefined,
  hint: undefined,
  position: '58',
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  file: 'scan.l',
  line: '1053',
  routine: 'scanner_yyerror',

Is there a way to fix this?

The following has my model definitions
Model: GoesVariable

'use strict';

module.exports = function(sequelize, DataTypes) {

  var GoesVariable = sequelize.define('GoesVariable', {
    variableName: {
      type: DataTypes.STRING,
      primaryKey: true
    },
    description: {
      type: DataTypes.TEXT,
      allowNull: true,
      validate: {
        notEmpty: true
      }
    }
  }, {
    classMethods: {
      associate: function(models) {
        GoesVariable.hasMany(models.GoesMap, {foreignKey: 'variableName'});
        GoesVariable.hasMany(models.GoesData, {foreignKey: 'variableName'});
      }
    }
  });

  return GoesVariable;
};

Model: GoesMap

'use strict';

module.exports = function(sequelize, DataTypes) {
  var GoesMap = sequelize.define('GoesMap', {
    variableName: {
      type: DataTypes.STRING,
      references: 'GoesVariables',
      referencesKey: 'variableName',
      primaryKey: true
    },
    imagePath: {
      type: DataTypes.STRING,
      primaryKey: true,
      validate: {
        notEmpty: true
      }
    },
    dataDate: {
      type: DataTypes.DATE,
      primaryKey: true
    },
    createdAt: {
      type: DataTypes.DATE,
      defaultValue: DataTypes.NOW,
      primaryKey: true
    },
  }, {
    classMethods: {
      associate: function(models) {
        GoesMap.belongsTo(models.GoesVariable, {foreignKey: 'variableName'});
      },
      getLatest: function(variableName) {
        return GoesMap.find({
          where: {
            variableName: variableName
          },
          order: 'dataDate desc'
        });
      },
      getMapsBetween: function(variableName,startDate, endDate) {
        return GoesMap.findAll({
          where: {
            variableName: variableName,
            dataDate: {
              between: [startDate, endDate]
            }
          }
        });
      }
    }
  });

  return GoesMap;
};
@janmeier

This comment has been minimized.

Member

janmeier commented Apr 3, 2014

Do you have quoteIdentifiers set to false? It seems that there are no quotes at all in the query you posted, so aliassing GoesMaps.variableName (table.column) to GoesMaps.variableName (string) fails because without quotes there is no way to distinquish the two.

I'm not saying that the fix is simply to remove quoteIdentifiers: false, since there are very legitimate reasons for having it if you're working with a legacy db - just want to make sure that's the error.

I guess that even though quouteIdentifiers is false, aliasses should still be quoted though.

@vjames19

This comment has been minimized.

vjames19 commented Apr 3, 2014

I'm using quoteIdentifiers: false. Which I need since changing the database to use quotes I would need to change other applications which right now use the database.

Is there a workaround for this?

@janmeier janmeier added the Bug label Apr 3, 2014

@janmeier

This comment has been minimized.

Member

janmeier commented Apr 3, 2014

Nope, you've uncovered a bug. The problem is that quoteIdentifiers removes ALL quotes, while it should still allow quotes around stuff that's aliased.
So basically it should be

GoesMaps.variableName AS "GoesMaps.variableName"
@vjames19

This comment has been minimized.

vjames19 commented Apr 3, 2014

How long would it take to fix this? Is it complex?

@janmeier

This comment has been minimized.

Member

janmeier commented Apr 4, 2014

Not really sure, but shouldn't be too complex.

If you can provide a PR with a couple of test cases it will be
significantly easier for one of the core team members to fix. You are also
welcome to try to provide a fix yourself, the relevant code should be in
lib/postgres/query-generator

@vjames19

This comment has been minimized.

vjames19 commented Apr 4, 2014

I will try to provide a PR for the tests in the following days.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented May 5, 2014

@vjames19 did you ever get around to creating that unit test?

@blakmatrix

This comment has been minimized.

blakmatrix commented Jul 31, 2014

I too am running into the same issue 👍

@idris

This comment has been minimized.

Contributor

idris commented May 20, 2015

@mickhansen I'm having this same issue. Was this ever fixed?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented May 21, 2015

@idris A case was never provided

idris added a commit to idris/sequelize that referenced this issue May 21, 2015

@idris

This comment has been minimized.

Contributor

idris commented May 21, 2015

I couldn't figure out how to write a unit test for this, but I fixed the problem (at least it fixed my use case)
#3780

IrfanBaqui pushed a commit to IrfanBaqui/sequelize that referenced this issue Jun 24, 2015

@ra50

This comment has been minimized.

ra50 commented Oct 18, 2015

This bug is marked as fixed in release 3.1.1 but the fix is incomplete. When doing a many-to-many join through a junction table, the alias name ends up being two "levels" deep, for instance Table2.joinTable, and this alias is not quoted. Other aliases are. The quoting needs applied to these aliases as well. In addition, the quoting needs to be used when the alias is used elsewhere to reference a column, for instance in the ON clause of the join.

This entire issue could also be avoided by not using "." in aliases in the first place. If it just used underscores instead, no quotes would be necessary and the SQL would be valid. Considering the aliases don't really matter, this could be an easier solution than inserting quotes even when quoteIdentifiers is set to false.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Oct 19, 2015

@ra50 underscores don't work since they can be a part of a legal field identifier (ex: first_name)

@ra50

This comment has been minimized.

ra50 commented Oct 28, 2015

I see how that could be an issue if the alias ended up matching an actual column somewhere. Either way, this bug should not be marked closed as the included fix is incomplete. Junction table aliases, and the reference of those aliases, need quoted as well.

@aray12

This comment has been minimized.

aray12 commented Jan 5, 2016

Is this being discussed any further? +1 for fix

@yjx3097890

This comment has been minimized.

yjx3097890 commented May 24, 2016

+1 for fix

@sequelize sequelize locked and limited conversation to collaborators May 24, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.