MINOR: Colliding field check runs twice #381

Merged
merged 1 commit into from May 2, 2012

Conversation

Projects
None yet
4 participants
@beerbohmdo
Contributor

beerbohmdo commented Apr 30, 2012

The check for colliding fields is already handled by DataQuery Class:
https://github.com/silverstripe/sapphire/blob/master/model/DataQuery.php#L195

What is the reason to check in the Database Abstraction Layer again for fields with the same name?
https://github.com/silverstripe/sapphire/blob/master/model/MySQLDatabase.php#L1162

@chillu

This comment has been minimized.

Show comment Hide comment
@chillu

chillu May 2, 2012

Member

@sminnee Looks good to me, field collision detection shouldn't be restricted to a specific DB driver anyway, right?

Member

chillu commented May 2, 2012

@sminnee Looks good to me, field collision detection shouldn't be restricted to a specific DB driver anyway, right?

@halkyon

This comment has been minimized.

Show comment Hide comment
@halkyon

halkyon May 2, 2012

Member

Can this be applied to any of the other database adapters?

Member

halkyon commented May 2, 2012

Can this be applied to any of the other database adapters?

sminnee added a commit that referenced this pull request May 2, 2012

Merge pull request #381 from AngryPHPNerd/patch-2
MINOR: Colliding field check runs twice

@sminnee sminnee merged commit a004acf into silverstripe:master May 2, 2012

@sminnee

This comment has been minimized.

Show comment Hide comment
@sminnee

sminnee May 2, 2012

Member

@halkyon The patch won't be applicable directly, but yes, you can assume in the other adapters that columns won't collide.

  • MSSQL: You can replace mssql_fetch_row with mssql_fetch_assoc. sqlsrv_fetch_array remains the same but you can use SQLSRV_FETCH_ASSOC instead of SQLSRV_FETCH_NUMERIC.
  • PostgreSQL: You can replace pg_fetch_row with pg_fetch_assoc.
Member

sminnee commented May 2, 2012

@halkyon The patch won't be applicable directly, but yes, you can assume in the other adapters that columns won't collide.

  • MSSQL: You can replace mssql_fetch_row with mssql_fetch_assoc. sqlsrv_fetch_array remains the same but you can use SQLSRV_FETCH_ASSOC instead of SQLSRV_FETCH_NUMERIC.
  • PostgreSQL: You can replace pg_fetch_row with pg_fetch_assoc.
@chillu

This comment has been minimized.

Show comment Hide comment
@chillu

chillu May 3, 2012

Member
Member

chillu commented May 3, 2012

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