Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Always return first (and only) value for "postgres_output value".

This is second part of the "write proper SQL queries" campaign. Queries
that return more than one value will result in "500 Internal Server Error"
response.
  • Loading branch information...
commit b8d1f045a32228c2c58ed929c0f66a5f7f6d05da 1 parent 61af6a7
@PiotrSikora PiotrSikora authored
View
5 README.md
@@ -89,7 +89,7 @@ This directive can be used more than once within same context.
postgres_output
---------------
-* **syntax**: `postgres_output rds|text|value|binary_value|none [row] [column]`
+* **syntax**: `postgres_output rds|text|value|binary_value|none`
* **default**: `rds`
* **context**: `http`, `server`, `location`, `if location`
@@ -107,9 +107,6 @@ Set output format:
extracting values with `postgres_set` for use with other modules (without
`Content-Type`).
-Row and column numbers start at 0. Column name can be used instead of column
-number.
-
postgres_set
------------
View
79 src/ngx_postgres_module.c
@@ -83,7 +83,7 @@ static ngx_command_t ngx_postgres_module_commands[] = {
{ ngx_string("postgres_output"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|
- NGX_HTTP_LIF_CONF|NGX_CONF_TAKE123,
+ NGX_HTTP_LIF_CONF|NGX_CONF_TAKE1,
ngx_postgres_conf_output,
NGX_HTTP_LOC_CONF_OFFSET,
0,
@@ -217,12 +217,12 @@ ngx_postgres_rewrite_enum_t ngx_postgres_rewrite_handlers[] = {
};
ngx_postgres_output_enum_t ngx_postgres_output_handlers[] = {
- { ngx_string("none"), 0, 0, NULL },
- { ngx_string("rds"), 0, 0, ngx_postgres_output_rds },
- { ngx_string("text") , 0, 0, ngx_postgres_output_text },
- { ngx_string("value"), 2, 0, ngx_postgres_output_value },
- { ngx_string("binary_value"), 2, 1, ngx_postgres_output_value },
- { ngx_null_string, 0, 0, NULL }
+ { ngx_string("none"), 0, NULL },
+ { ngx_string("rds"), 0, ngx_postgres_output_rds },
+ { ngx_string("text") , 0, ngx_postgres_output_text },
+ { ngx_string("value"), 0, ngx_postgres_output_value },
+ { ngx_string("binary_value"), 1, ngx_postgres_output_value },
+ { ngx_null_string, 0, NULL }
};
@@ -309,7 +309,7 @@ ngx_postgres_create_loc_conf(ngx_conf_t *cf)
* conf->query.methods_set = 0
* conf->query.methods = NULL
* conf->query.def = NULL
- * conf->output_value = NULL
+ * conf->output_binary = 0
*/
conf->upstream.connect_timeout = NGX_CONF_UNSET_MSEC;
@@ -368,11 +368,11 @@ ngx_postgres_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
if (prev->output_handler == NGX_CONF_UNSET_PTR) {
/* default */
conf->output_handler = ngx_postgres_output_rds;
- conf->output_value = NULL;
+ conf->output_binary = 0;
} else {
/* merge */
conf->output_handler = prev->output_handler;
- conf->output_value = prev->output_value;
+ conf->output_binary = prev->output_binary;
}
}
@@ -1010,64 +1010,7 @@ ngx_postgres_conf_output(ngx_conf_t *cf, ngx_command_t *cmd, void *conf)
return NGX_CONF_ERROR;
}
- if (e[i].args != cf->args->nelts - 2) {
- ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
- "postgres: invalid number of arguments"
- " in \"%V\" directive", &cmd->name);
-
- dd("returning NGX_CONF_ERROR");
- return NGX_CONF_ERROR;
- }
-
- if (e[i].args == 0) {
- dd("returning NGX_CONF_OK");
- return NGX_CONF_OK;
- }
-
- pglcf->output_value = ngx_palloc(cf->pool, sizeof(ngx_postgres_value_t));
- if (pglcf->output_value == NULL) {
- dd("returning NGX_CONF_ERROR");
- return NGX_CONF_ERROR;
- }
-
- pglcf->output_value->binary = e[i].binary;
-
- pglcf->output_value->row = ngx_atoi(value[2].data, value[2].len);
- if (pglcf->output_value->row == NGX_ERROR) {
- ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
- "postgres: invalid row number \"%V\""
- " in \"%V\" directive", &value[2], &cmd->name);
-
- dd("returning NGX_CONF_ERROR");
- return NGX_CONF_ERROR;
- }
-
- if (e[i].args == 1) {
- dd("returning NGX_CONF_OK");
- return NGX_CONF_OK;
- }
-
- if (value[3].len == 0) {
- ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
- "postgres: empty column in \"%V\" directive",
- &cmd->name);
-
- dd("returning NGX_CONF_ERROR");
- return NGX_CONF_ERROR;
- }
-
- pglcf->output_value->column = ngx_atoi(value[3].data, value[3].len);
- if (pglcf->output_value->column == NGX_ERROR) {
- /* get column by name */
- pglcf->output_value->col_name = ngx_pnalloc(cf->pool, value[3].len + 1);
- if (pglcf->output_value->col_name == NULL) {
- dd("returning NGX_CONF_ERROR");
- return NGX_CONF_ERROR;
- }
-
- (void) ngx_cpystrn(pglcf->output_value->col_name,
- value[3].data, value[3].len + 1);
- }
+ pglcf->output_binary = e[i].binary;
dd("returning NGX_CONF_OK");
return NGX_CONF_OK;
View
6 src/ngx_postgres_module.h
@@ -59,7 +59,6 @@ typedef struct {
ngx_int_t column;
u_char *col_name;
ngx_uint_t required;
- unsigned binary:1;
} ngx_postgres_value_t;
typedef struct {
@@ -96,11 +95,10 @@ typedef struct {
} ngx_postgres_rewrite_enum_t;
typedef ngx_int_t (*ngx_postgres_output_handler_pt)
- (ngx_http_request_t *, PGresult *, ngx_postgres_value_t *);
+ (ngx_http_request_t *, PGresult *);
typedef struct {
ngx_str_t name;
- ngx_uint_t args;
unsigned binary:1;
ngx_postgres_output_handler_pt handler;
} ngx_postgres_output_enum_t;
@@ -160,7 +158,7 @@ typedef struct {
ngx_array_t *rewrites;
/* output */
ngx_postgres_output_handler_pt output_handler;
- ngx_postgres_value_t *output_value;
+ unsigned output_binary:1;
/* custom variables */
ngx_array_t *variables;
} ngx_postgres_loc_conf_t;
View
48 src/ngx_postgres_output.c
@@ -33,58 +33,32 @@
ngx_int_t
-ngx_postgres_output_value(ngx_http_request_t *r, PGresult *res,
- ngx_postgres_value_t *pgv)
+ngx_postgres_output_value(ngx_http_request_t *r, PGresult *res)
{
ngx_postgres_ctx_t *pgctx;
ngx_http_core_loc_conf_t *clcf;
ngx_chain_t *cl;
ngx_buf_t *b;
size_t size;
- ngx_int_t col_count, row_count, col;
dd("entering");
pgctx = ngx_http_get_module_ctx(r, ngx_postgres_module);
- col_count = pgctx->var_cols;
- row_count = pgctx->var_rows;
-
- if (pgv->column != NGX_ERROR) {
- /* get column by number */
- col = pgv->column;
- } else {
- /* get column by name */
- col = PQfnumber(res, (char const *) pgv->col_name);
- if (col == NGX_ERROR) {
- clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
-
- ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
- "postgres: \"postgres_output value\" requires"
- " value from column \"%s\" that wasn't found"
- " in the received result-set in location \"%V\"",
- pgv->col_name, &clcf->name);
-
- dd("returning NGX_DONE, status NGX_HTTP_INTERNAL_SERVER_ERROR");
- pgctx->status = NGX_HTTP_INTERNAL_SERVER_ERROR;
- return NGX_DONE;
- }
- }
-
- if ((pgv->row >= row_count) || (col >= col_count)) {
+ if ((pgctx->var_rows != 1) || (pgctx->var_cols != 1)) {
clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
- "postgres: \"postgres_output value\" requires value out"
- " of range of the received result-set (rows:%d cols:%d)"
- " in location \"%V\"", row_count, col_count, &clcf->name);
+ "postgres: \"postgres_output value\" received %d value(s)"
+ " instead of expected single value in location \"%V\"",
+ pgctx->var_rows * pgctx->var_cols, &clcf->name);
dd("returning NGX_DONE, status NGX_HTTP_INTERNAL_SERVER_ERROR");
pgctx->status = NGX_HTTP_INTERNAL_SERVER_ERROR;
return NGX_DONE;
}
- if (PQgetisnull(res, pgv->row, col)) {
+ if (PQgetisnull(res, 0, 0)) {
clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
@@ -96,7 +70,7 @@ ngx_postgres_output_value(ngx_http_request_t *r, PGresult *res,
return NGX_DONE;
}
- size = PQgetlength(res, pgv->row, col);
+ size = PQgetlength(res, 0, 0);
if (size == 0) {
clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
@@ -125,7 +99,7 @@ ngx_postgres_output_value(ngx_http_request_t *r, PGresult *res,
b->memory = 1;
b->tag = r->upstream->output.tag;
- b->last = ngx_copy(b->last, PQgetvalue(res, pgv->row, col), size);
+ b->last = ngx_copy(b->last, PQgetvalue(res, 0, 0), size);
if (b->last != b->end) {
dd("returning NGX_ERROR");
@@ -142,8 +116,7 @@ ngx_postgres_output_value(ngx_http_request_t *r, PGresult *res,
}
ngx_int_t
-ngx_postgres_output_text(ngx_http_request_t *r, PGresult *res,
- ngx_postgres_value_t *pgv)
+ngx_postgres_output_text(ngx_http_request_t *r, PGresult *res)
{
ngx_postgres_ctx_t *pgctx;
ngx_chain_t *cl;
@@ -227,8 +200,7 @@ ngx_postgres_output_text(ngx_http_request_t *r, PGresult *res,
}
ngx_int_t
-ngx_postgres_output_rds(ngx_http_request_t *r, PGresult *res,
- ngx_postgres_value_t *pgv)
+ngx_postgres_output_rds(ngx_http_request_t *r, PGresult *res)
{
ngx_postgres_ctx_t *pgctx;
ngx_chain_t *first, *last;
View
9 src/ngx_postgres_output.h
@@ -37,12 +37,9 @@
#include "resty_dbd_stream.h"
-ngx_int_t ngx_postgres_output_value(ngx_http_request_t *, PGresult *,
- ngx_postgres_value_t *);
-ngx_int_t ngx_postgres_output_text(ngx_http_request_t *, PGresult *,
- ngx_postgres_value_t *);
-ngx_int_t ngx_postgres_output_rds(ngx_http_request_t *, PGresult *,
- ngx_postgres_value_t *);
+ngx_int_t ngx_postgres_output_value(ngx_http_request_t *, PGresult *);
+ngx_int_t ngx_postgres_output_text(ngx_http_request_t *, PGresult *);
+ngx_int_t ngx_postgres_output_rds(ngx_http_request_t *, PGresult *);
ngx_chain_t *ngx_postgres_render_rds_header(ngx_http_request_t *,
ngx_pool_t *, PGresult *, ngx_int_t, ngx_int_t);
ngx_chain_t *ngx_postgres_render_rds_columns(ngx_http_request_t *,
View
4 src/ngx_postgres_processor.c
@@ -222,7 +222,7 @@ ngx_postgres_upstream_send_query(ngx_http_request_t *r, ngx_connection_t *pgxc,
dd("sending query: %s", query);
- if (pglcf->output_value && pglcf->output_value->binary) {
+ if (pglcf->output_binary) {
pgrc = PQsendQueryParams(pgdt->pgconn, (const char *) query,
0, NULL, NULL, NULL, NULL, /* binary */ 1);
} else {
@@ -392,7 +392,7 @@ ngx_postgres_process_response(ngx_http_request_t *r, PGresult *res)
if (pglcf->output_handler) {
/* generate output */
dd("returning");
- return pglcf->output_handler(r, res, pglcf->output_value);
+ return pglcf->output_handler(r, res);
}
dd("returning NGX_DONE");
View
4 t/eval.t
@@ -29,7 +29,7 @@ __DATA__
eval $backend {
postgres_pass database;
postgres_query "select '$scheme://127.0.0.1:$server_port/echo'";
- postgres_output value 0 0;
+ postgres_output value;
}
proxy_pass $backend;
@@ -59,7 +59,7 @@ it works!
eval $echo {
postgres_pass database;
postgres_query "select 'test' as echo";
- postgres_output value 0 0;
+ postgres_output value;
}
echo -n $echo;
View
119 t/output.t
@@ -46,8 +46,8 @@ GET /postgres
location /postgres {
postgres_pass database;
- postgres_query "select 'test', 'test' as echo";
- postgres_output value 0 0;
+ postgres_query "select 'test' as echo";
+ postgres_output value;
}
--- request
GET /postgres
@@ -67,8 +67,8 @@ test
location /postgres {
postgres_pass database;
- postgres_query "select 'test', 'test' as echo";
- postgres_output value 0 1;
+ postgres_query "select 'test' as echo";
+ postgres_output value;
}
--- request
GET /postgres
@@ -81,23 +81,6 @@ test
-=== TEST 4: value - value outside of the result-set
---- http_config eval: $::http_config
---- config
- default_type text/plain;
-
- location /postgres {
- postgres_pass database;
- postgres_query "select 'test' as echo";
- postgres_output value 2 2;
- }
---- request
-GET /postgres
---- error_code: 500
---- timeout: 10
-
-
-
=== TEST 5: value - NULL value
--- http_config eval: $::http_config
--- config
@@ -106,7 +89,7 @@ GET /postgres
location /postgres {
postgres_pass database;
postgres_query "select NULL as echo";
- postgres_output value 0 0;
+ postgres_output value;
}
--- request
GET /postgres
@@ -123,7 +106,7 @@ GET /postgres
location /postgres {
postgres_pass database;
postgres_query "select '' as echo";
- postgres_output value 0 0;
+ postgres_output value;
}
--- request
GET /postgres
@@ -234,11 +217,11 @@ Content-Type: application/x-resty-dbd-stream
--- http_config eval: $::http_config
--- config
default_type text/plain;
- postgres_output value 0 3;
+ postgres_output value;
location /postgres {
postgres_pass database;
- postgres_query "select 'a', 'b', 'c', 'test' as echo";
+ postgres_query "select 'test' as echo";
}
--- request
GET /postgres
@@ -272,44 +255,6 @@ GET /postgres
-=== TEST 12: value - column by name (existing)
---- http_config eval: $::http_config
---- config
- default_type text/plain;
-
- location /postgres {
- postgres_pass database;
- postgres_query "select 'test' as echo";
- postgres_output value 0 "echo";
- }
---- request
-GET /postgres
---- error_code: 200
---- response_headers
-Content-Type: text/plain
---- response_body chomp
-test
---- timeout: 10
-
-
-
-=== TEST 13: value - column by name (not existing)
---- http_config eval: $::http_config
---- config
- default_type text/plain;
-
- location /postgres {
- postgres_pass database;
- postgres_query "select 'test' as echo";
- postgres_output value 0 "test";
- }
---- request
-GET /postgres
---- error_code: 500
---- timeout: 10
-
-
-
=== TEST 14: value - sanity (request with known extension)
--- http_config eval: $::http_config
--- config
@@ -317,8 +262,8 @@ GET /postgres
location /postgres {
postgres_pass database;
- postgres_query "select 'test', 'test' as echo";
- postgres_output value 0 0;
+ postgres_query "select 'test' as echo";
+ postgres_output value;
}
--- request
GET /postgres.jpg
@@ -339,7 +284,7 @@ test
location /postgres {
postgres_pass database;
postgres_query "select E'\\001'::bytea as res";
- postgres_output value 0 0;
+ postgres_output value;
}
--- request
GET /postgres
@@ -360,7 +305,7 @@ Content-Type: text/plain
location /postgres {
postgres_pass database;
postgres_query "select E'\\001'::bytea as res";
- postgres_output binary_value 0 0;
+ postgres_output binary_value;
}
--- request
GET /postgres
@@ -381,7 +326,7 @@ Content-Type: text/plain
location /postgres {
postgres_pass database;
postgres_query "select 3::int2 as res";
- postgres_output binary_value 0 0;
+ postgres_output binary_value;
}
--- request
GET /postgres
@@ -402,8 +347,8 @@ Content-Type: text/plain
location /postgres {
if ($arg_foo) {
postgres_pass database;
- postgres_query "select * from cats order by id";
- postgres_output value 0 0;
+ postgres_query "select id from cats order by id limit 1";
+ postgres_output value;
break;
}
@@ -465,3 +410,37 @@ Content-Type: text/plain
--- response_body eval
""
--- timeout: 10
+
+
+
+=== TEST 21: value - empty result
+--- http_config eval: $::http_config
+--- config
+ default_type text/plain;
+
+ location /postgres {
+ postgres_pass database;
+ postgres_query "select * from cats where id=1";
+ postgres_output value;
+ }
+--- request
+GET /postgres
+--- error_code: 500
+--- timeout: 10
+
+
+
+=== TEST 22: value - too many values
+--- http_config eval: $::http_config
+--- config
+ default_type text/plain;
+
+ location /postgres {
+ postgres_pass database;
+ postgres_query "select * from cats";
+ postgres_output value;
+ }
+--- request
+GET /postgres
+--- error_code: 500
+--- timeout: 10
Please sign in to comment.
Something went wrong with that request. Please try again.