Download & Extend

mail_logger module failing to install with postgresql

Project:Mail Logger
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs review

Issue Summary

In the mail_logger table, "from" and "to" was not a good choices for column names under PostgreSQL. Need some modifications in this module:

- eliminate all back ticks from the install and the module
- escape these column names everywhere with double quote (install and module)
- remove these column form ordering in mail_logger_overview

A working version under Postgres is attached.

AttachmentSize
mail_logger.zip2.96 KB

Comments

#1

Status:active» needs work

Seems like there ought to be a better way to escape those queries. They basically look like this in the attached zip:

<?php
db_query
("INSERT INTO {mail_logger} (
  \"mailkey\" , \"to\" , \"subject\" , \"body\" , \"from\" , \"headers\", \"date_sent\", \"language\"
  ) VALUES (
  '%s', '%s', '%s', '%s', '%s', '%s', %d, '%s' )"
, $args);

$sql = "SELECT ml.\"mlid\", ml.\"mailkey\", ml.\"to\", ml.\"from\", ml.\"subject\", ml.\"date_sent\" FROM {mail_logger} ml";

$result = pager_query($sql ." WHERE ml.\"mailkey\" = '%s'". $tablesort, 50, 0, NULL, $type);
?>

It doesn't look quite so weird in the .install file, but isn't the standard for escaping column names to use backticks, not double quotes anyway or does that not work the same in PostgreSQL as in MySQL? If not backticks, why not apostrophes?

#2

Status:needs work» closed (fixed)

So... close.

#3

Version:6.x-1.0» 6.x-1.x-dev
Status:closed (fixed)» needs review

Reopening, as i've stumbled upon this issue.

Here's a diff against last cvs snapshot that fixes the issue for me:
- rename to -> mail_to and from -> mail_from. to and from are sql keywords, so regardless of quoting this can lead to confusion
- remove backticks, they are now unneeded, and they only work on mysql. On postgres, quoting is done using "".

the only thing not dealt with is the upgrade path for mysql users, where the existing db columns would need to be renamed in mail_logger_update_XXXX().

AttachmentSize
mail_logger.diff 5.28 KB

#4

Title:mail_logger module is failure under postres» mail_logger module failing to install with postgresql

#5

On top of that patch, here's another one needed for postgresql. On mysql, you can feed NULL to a serial field, on postgresql it's an error. So let's get the next value for serial using SELECT nextval('{mail_logger_mlid_seq}'). Tested here, works fine.

AttachmentSize
mail_logger_2.diff 1.39 KB