1

Foreword: I tried to find an answer to this and while there are some similarities in some of the answered questions, there were no matches.

I'm trying to generate 3 month reports but only the current date is returning a value.

<?php
// Db connect
include_once ('db.php');
global $con2;

// 3 month reports
$date_from = date("d-m-Y", strtotime(" -3 months"));
$date_to = date('d-m-Y');

// Count 
$get = "select * from table1 where date between '$date_from' and '$date_to'";
$get_connect = mysqli_query($con2, $get);
$get_rows = mysqli_num_rows($get_connect);

// Display data
echo $get_rows;
?>

Additional info. The dates already stored in the date column of table1 are in exactly the same format as the variables eg: 10-10-2017 The entires which are from today are being displayed but nothing else is. Is this because PHP can only read up till the first - dash?

  • 1
    Why aren't you using a `date` datatype for dates in your database? – Mark Baker Nov 20 '17 at 21:30
  • It's using InnoDB as and utf8_general_ci the only default options for that are current time stamp and we need two separate columns to record time and date. –  Nov 20 '17 at 21:35
  • Why? That makes no sense.... innodb has standard [date/datetime datatypes](https://dev.mysql.com/doc/refman/5.7/en/date-and-time-types.html).... why are you using a varchar and making your life unnecessarily complex – Mark Baker Nov 20 '17 at 21:36
  • 3
    if you change your code and put hardcoded dates, the query works fine or not? Just to localize where the error is. – Gabriel Souto Nov 20 '17 at 21:37
  • The system is already in operation and has been for some time. There are many records so changing table structure is not possible. Can you help with the question? –  Nov 20 '17 at 21:38
  • 2
    The error is assuming that `20-11-2017` is greater than `21-09-2017` when comparing strings – Mark Baker Nov 20 '17 at 21:39
  • 1
    [Little Bobby](http://bobby-tables.com/) says **[you may be at risk for SQL Injection Attacks](https://stackoverflow.com/q/60174/)**. Learn about [Prepared Statements](https://en.wikipedia.org/wiki/Prepared_statement) with [parameterized queries](https://stackoverflow.com/a/4712113/5827005). I recommend `PDO`, which I [wrote a class for](https://github.com/GrumpyCrouton/GrumpyPDO) to make it extremely easy, clean, and more secure than using non-parameterized queries. Also, [This article](https://phpdelusions.net/pdo/mysqli_comparison) may help you choose between `MySQLi` and `PDO` – GrumpyCrouton Nov 20 '17 at 21:40
  • 1
    If you don't change to sensible date/time datatypes, you'll constantly be having problems.... as a workround, you can cast those strings to dates using MySQL's [STR_TO_DATE()](https://dev.mysql.com/doc/refman/5.7/en/date-and-time-functions.html#function_str-to-date) function.... between will work if you're using date values, but not with strings formatted the way you've formatted them – Mark Baker Nov 20 '17 at 21:41
  • Gabriel Souto, thanks, I tried hard-coded dates and it works perfectly which is strange as I echoed out the variables and they work as well. No sure what is going on. –  Nov 20 '17 at 21:42
  • 3
    But I really recommend taking the time and effort to change your datatypes – Mark Baker Nov 20 '17 at 21:44
  • 2
    @GrumpyCrouton Even though I agree with that Prepared Statements are the preferred way to go (and a _must_ when working with unknown data like user inputs etc), the above script is actually _not_ at risk for SQL injections. Unless someone manages to overload the `date()` function that is, but then you would have bigger issues. I just want to make that distinction. – M. Eriksson Nov 20 '17 at 21:47
  • @MagnusEriksson I agree with you that this code specifically is not at risk, but if someone is using concatenation for queries in this place, I feel as though they could be doing it _elsewhere_ (And may not realize how bad it is), which may put them at risk, and is also the reason my copy-paste message from before says "you may be at risk" instead of what it used to say; "you are at risk" :) – GrumpyCrouton Nov 20 '17 at 21:49
  • Looks like you dont need to put 'month' in plural, according to php doc – Gabriel Souto Nov 20 '17 at 21:50
  • @GrumpyCrouton I understand why, but sometimes it can be a bad thing calling wolf before the wolf is coming. It might have the effect that people read about it and realizing that it's not an issue in their script, and therefor discarding it completely. Anyway, I agree with Prepared Statements and it's a good thing to point it out. It's just the _"is at risk"_-part that I think should be used carefully. And this is a bit off-topic :-) – M. Eriksson Nov 20 '17 at 21:52
  • @Gabriel Souto can you provide a link so I can understand what you mean? Thanks. –  Nov 20 '17 at 21:53
  • @MagnusEriksson I guess my thinking is, they should be used even when there may not be any danger in a specific query. Mainly because of consistency within your code, and you never know what vulnerabilities you may have. But I think this is a discussion for another time/place :) – GrumpyCrouton Nov 20 '17 at 21:54
  • @Steven sure, http://php.net/manual/pt_BR/function.strtotime.php, all examples using month are in singular, -3 month, +2 month, etc – Gabriel Souto Nov 20 '17 at 21:56
  • You can fix this forever in 2 minutes. Add two new columns with date datatypes and then run `UPDATE table SET newdatefrom = STR_TO_DATE(datefrom,'%d-%m-%Y') WHERE id < 10000` once for each column – miknik Nov 20 '17 at 22:07
  • Thanks Gabriel, I tried changing "months" to "month" and it's just the same. Both plural and singular echo out the correct value but neither display the range and therefore the rows = 0. –  Nov 20 '17 at 22:07

1 Answers1

2

Using dd-mm-yyyy strings as dates simply will not work correctly with "between"

select
*
from (
    select '10-10-2017' as 'date' union all
    select '11-11-2017' as 'date' union all
    select '10-10-1989' as 'date' union all
    select '10-10-2020' as 'date' union all
    select '10-10-3017' as 'date'
    ) table1
where `date` between '10-10-2017' and '11-11-2017'
order by `date`

    date
1   10-10-2017
2   10-10-2020
3   10-10-3017
4   11-11-2017

but conversion to a real date does:

select
* , str_to_date(`date`,'%d-%m-%Y') 
from (
    select '10-10-2017' as 'date' union all
    select '11-11-2017' as 'date' union all
    select '10-10-1989' as 'date' union all
    select '10-10-2020' as 'date' union all
    select '10-10-3017' as 'date'
    ) table1
where str_to_date(`date`,'%d-%m-%Y') between str_to_date('10-10-2017','%d-%m-%Y')  and str_to_date('11-11-2017','%d-%m-%Y') 
order by str_to_date(`date`,'%d-%m-%Y') 


    date        str_to_date(`date`,'%d-%m-%Y')
1   10-10-2017  10.10.2017 00:00:00
2   11-11-2017  11.11.2017 00:00:00

Demo

In addition: The way "between" is implemented in SQL is that the first value must be lower than the second value (i.e. the sequence of the value IS vital). That is because "between" is really just a syntax shortcut for somevalue >= compare_val_low and somevalue <= compare_val_high if you reverse the comparison values it will return NULL unless somevalue happens to equal one or both of the comparison values. Using strings it will be easy for what appears to be a lower date, to in fact be a "higher value" due to the nature of string ordering, e.g. this might "look" ok:

between '29-08-2017' and '11-11-2017' 

but it is not OK at all because '29' is higher than '11' and between will return no rows.

Paul Maxwell
  • 33,002
  • 3
  • 32
  • 51
  • I see. Thanks for example. I guess the only way of doing this with variables would be to change the format of the date to yyyy-mm-dd –  Nov 21 '17 at 22:38