Business Scenario AIrline#33
Conversation
ghost
left a comment
There was a problem hiding this comment.
Excellent work! See comments, fix and resubmit.
| create table dbo.Booking( | ||
| BookingId int not null identity primary key, | ||
| FlightNum char(6) not null | ||
| constraint ck_Booking_flight_number_must_start_with_Fly_then_fllowed_by_3_numbers check(FlightNum like 'Fly[0-9][0-9][0-9]'), |
There was a problem hiding this comment.
Changed constraint
I have now 1 constraint that enforces that FilghtNum Should:
- begin with fly
- Then be followed by 3 digits
- that is not 000 (I wouldn't use between since 000 is the only wrong input here)
Since all those constraints are formatting rules for FlightNum is it better to keep it in one big constraint so I would get all the instructions/errors the first time rather then keep getting errors until all's god?
There was a problem hiding this comment.
You should only have one of each type of constraint on each column. Example if you have multiple check constraints on single column, you should always combine them. That's why sql gives error when you have multiple check constraints. Splitting them with commas makes them multi column constraints.
| PassportNum varchar(9) null | ||
| constraint ck_Booking_passport_num_cannot_start_with_0 check(PassportNum not like '0%'), | ||
| constraint ck_Booking_passport_num_length_must_be_9 check(len(PassportNum) = 9), | ||
| constraint ck_Booking_passport_num_can_only_be_numbers check(PassportNum not like '%[^0-9]%'), |
There was a problem hiding this comment.
When having multiple check constraints. Instead of making them multi column constraints, combine them together with and/or.
And you don't need first constraint, last is constraining it not to allow starting with 0.
There was a problem hiding this comment.
Could you please help me understand how the last constraint is enforcing the first digit should not be a 0?
And here I got the answer to my question for the previous comment
| constraint ck_Booking_passport_num_length_must_be_9 check(len(PassportNum) = 9), | ||
| constraint ck_Booking_passport_num_can_only_be_numbers check(PassportNum not like '%[^0-9]%'), | ||
| PassportIssueDate date null, | ||
| PassportExpiryDate as case |
There was a problem hiding this comment.
The case should be a constraint. It doesn't make sense to have expiry date as computed column.
There was a problem hiding this comment.
Why have constraints making errors when you can have it work automatic and save future work potential constraints/errors? Since expiry date is entirely based on issue date and passenger's DOB.
I guess I'm asking in general, what's the best practice?
There was a problem hiding this comment.
Because expiry date is data you add to the ticket. And it has to do with the passport not with the ticket. It's not necessarily based on the other dates.
On a side note. If you have a passport that's still valid, and you make a new one, you will have extra dates on your passport but if it's > 10 years old at time of travel, you won't be able to travel even if it's still valid.
| constraint ck_Booking_passport_issue_date_cannot_be_before_passenger_dob check(PassengerDOB <= PassportIssueDate), | ||
| constraint ck_Booking_passport_booked_date_cannot_be_before_passport_issue_date check(PassportIssueDate <= BookedDate), | ||
| constraint ck_Booking_checked_in_time_cannot_be_before_booked_date check((BookedDate <= CheckedInTime) or CheckedInTime is null), | ||
| constraint ck_Booking_passenger_age_must_be_between_16_and_90 |
There was a problem hiding this comment.
I only want to consider it a full year when the day and month have also passed
According to SQL select datediff(year, '2022-12-31', '2023-01-01') is 1 year apert when in reality it's 1 day apert
|
|
||
| --All or non constraints | ||
| constraint ck_Booking_passport_num_passport_issue_date_passport_nationality_and_checked_in_time_must_all_either_be_completed_or_null | ||
| check((PassportNum is null and PassportIssueDate is null and PassportNationality is null and CheckedInTime is null) |
There was a problem hiding this comment.
You can have passport details even if not checked in yet.
There was a problem hiding this comment.
"Aren't they working hand in hand? According to the spec, 'When you book the flight you would only need to provide your name, DOB, and address, but in order to be checked in and travel, we need the passport details.' So, I understood that when you check in, you give your passport details, and it's one process."
There was a problem hiding this comment.
Not necessarily. You can add the passport details before. At the time you check in, you must have the details.
| or (PassportNum is not null and PassportIssueDate is not null and PassportNationality is not null and CheckedInTime is not null)), | ||
|
|
||
| --Unique Constraints | ||
| constraint u_Booking_one_passenger_cannot_book_2_tickets_on_the_same_flight unique (FlightNum, PassengerName, PassengerDOB, PassengerAddress), |
There was a problem hiding this comment.
I fixed it by adding DepartureTime to the unique constraint, so now Passenger Info FlightNum & DepartureTime must be unique
| group by b.FlightNum, b.DepartureAirport, b.DepartureTime, b.ArrivalAirport | ||
|
|
||
| --2) Who isn't checked in for flights departing in the next week, in order to send them reminders to check in. | ||
| select * |
There was a problem hiding this comment.
Added datediff(day, getdate(), b.DepartureTime) between 0 and 7 to the where clause
| group by convert(date, b.DepartureTime, 23) | ||
|
|
||
| --4) How many flights are departing per destination, and num of passengers we have on those flights, to know what route is attracts most people. | ||
| select b.DepartureAirport, Flights = b.FlightNum, Passengers = count(*) |
|
|
||
| --6) How many flights does each person have (to know for frequent flyer status), as: last name, first name, number of flights. | ||
| select LastName = substring(b.PassengerName,charindex(' ', b.PassengerName)+1, len(b.PassengerName)), | ||
| FirstName = substring(b.PassengerName, 1, charindex(' ', b.PassengerName)-1), |
There was a problem hiding this comment.
If you need to do substring() maybe consider splitting first and last name in data
There was a problem hiding this comment.
Can you please elaborate what mean by "splitting first and last name in data"
There was a problem hiding this comment.
Instead of having full name as one column, use 2 columns one for first name and one for last name.
There was a problem hiding this comment.
That's how my code was initially
There was a problem hiding this comment.
And why did you change it? You should always have first and last name split.
jacobglanz
left a comment
There was a problem hiding this comment.
Please review my comments, and i have committed my changes up until here.
| create table dbo.Booking( | ||
| BookingId int not null identity primary key, | ||
| FlightNum char(6) not null | ||
| constraint ck_Booking_flight_number_must_start_with_Fly_then_fllowed_by_3_numbers check(FlightNum like 'Fly[0-9][0-9][0-9]'), |
There was a problem hiding this comment.
Changed constraint
I have now 1 constraint that enforces that FilghtNum Should:
- begin with fly
- Then be followed by 3 digits
- that is not 000 (I wouldn't use between since 000 is the only wrong input here)
Since all those constraints are formatting rules for FlightNum is it better to keep it in one big constraint so I would get all the instructions/errors the first time rather then keep getting errors until all's god?
| PassportNum varchar(9) null | ||
| constraint ck_Booking_passport_num_cannot_start_with_0 check(PassportNum not like '0%'), | ||
| constraint ck_Booking_passport_num_length_must_be_9 check(len(PassportNum) = 9), | ||
| constraint ck_Booking_passport_num_can_only_be_numbers check(PassportNum not like '%[^0-9]%'), |
There was a problem hiding this comment.
Could you please help me understand how the last constraint is enforcing the first digit should not be a 0?
And here I got the answer to my question for the previous comment
| constraint ck_Booking_passport_num_length_must_be_9 check(len(PassportNum) = 9), | ||
| constraint ck_Booking_passport_num_can_only_be_numbers check(PassportNum not like '%[^0-9]%'), | ||
| PassportIssueDate date null, | ||
| PassportExpiryDate as case |
There was a problem hiding this comment.
Why have constraints making errors when you can have it work automatic and save future work potential constraints/errors? Since expiry date is entirely based on issue date and passenger's DOB.
I guess I'm asking in general, what's the best practice?
| constraint ck_Booking_passport_issue_date_cannot_be_before_passenger_dob check(PassengerDOB <= PassportIssueDate), | ||
| constraint ck_Booking_passport_booked_date_cannot_be_before_passport_issue_date check(PassportIssueDate <= BookedDate), | ||
| constraint ck_Booking_checked_in_time_cannot_be_before_booked_date check((BookedDate <= CheckedInTime) or CheckedInTime is null), | ||
| constraint ck_Booking_passenger_age_must_be_between_16_and_90 |
There was a problem hiding this comment.
I only want to consider it a full year when the day and month have also passed
According to SQL select datediff(year, '2022-12-31', '2023-01-01') is 1 year apert when in reality it's 1 day apert
|
|
||
| --All or non constraints | ||
| constraint ck_Booking_passport_num_passport_issue_date_passport_nationality_and_checked_in_time_must_all_either_be_completed_or_null | ||
| check((PassportNum is null and PassportIssueDate is null and PassportNationality is null and CheckedInTime is null) |
There was a problem hiding this comment.
"Aren't they working hand in hand? According to the spec, 'When you book the flight you would only need to provide your name, DOB, and address, but in order to be checked in and travel, we need the passport details.' So, I understood that when you check in, you give your passport details, and it's one process."
| or (PassportNum is not null and PassportIssueDate is not null and PassportNationality is not null and CheckedInTime is not null)), | ||
|
|
||
| --Unique Constraints | ||
| constraint u_Booking_one_passenger_cannot_book_2_tickets_on_the_same_flight unique (FlightNum, PassengerName, PassengerDOB, PassengerAddress), |
There was a problem hiding this comment.
I fixed it by adding DepartureTime to the unique constraint, so now Passenger Info FlightNum & DepartureTime must be unique
| group by b.FlightNum, b.DepartureAirport, b.DepartureTime, b.ArrivalAirport | ||
|
|
||
| --2) Who isn't checked in for flights departing in the next week, in order to send them reminders to check in. | ||
| select * |
There was a problem hiding this comment.
Added datediff(day, getdate(), b.DepartureTime) between 0 and 7 to the where clause
| group by convert(date, b.DepartureTime, 23) | ||
|
|
||
| --4) How many flights are departing per destination, and num of passengers we have on those flights, to know what route is attracts most people. | ||
| select b.DepartureAirport, Flights = b.FlightNum, Passengers = count(*) |
|
|
||
| --6) How many flights does each person have (to know for frequent flyer status), as: last name, first name, number of flights. | ||
| select LastName = substring(b.PassengerName,charindex(' ', b.PassengerName)+1, len(b.PassengerName)), | ||
| FirstName = substring(b.PassengerName, 1, charindex(' ', b.PassengerName)-1), |
There was a problem hiding this comment.
Can you please elaborate what mean by "splitting first and last name in data"
ghost
left a comment
There was a problem hiding this comment.
Excellent! See comments, fix and resubmit.
| constraint ck_Booking_passport_num_length_must_be_9 check(len(PassportNum) = 9), | ||
| constraint ck_Booking_passport_num_can_only_be_numbers check(PassportNum not like '%[^0-9]%'), | ||
| PassportIssueDate date null, | ||
| PassportExpiryDate as case |
There was a problem hiding this comment.
Because expiry date is data you add to the ticket. And it has to do with the passport not with the ticket. It's not necessarily based on the other dates.
On a side note. If you have a passport that's still valid, and you make a new one, you will have extra dates on your passport but if it's > 10 years old at time of travel, you won't be able to travel even if it's still valid.
|
|
||
| --All or non constraints | ||
| constraint ck_Booking_passport_num_passport_issue_date_passport_nationality_and_checked_in_time_must_all_either_be_completed_or_null | ||
| check((PassportNum is null and PassportIssueDate is null and PassportNationality is null and CheckedInTime is null) |
There was a problem hiding this comment.
Not necessarily. You can add the passport details before. At the time you check in, you must have the details.
| constraint ck_Booking_passport_issue_date_cannot_be_before_passenger_dob check(PassengerDOB <= PassportIssueDate), | ||
| constraint ck_Booking_passport_booked_date_cannot_be_before_passport_issue_date check(PassportIssueDate <= BookedDate), | ||
| constraint ck_Booking_checked_in_time_cannot_be_before_booked_date check((BookedDate <= CheckedInTime) or CheckedInTime is null), | ||
| constraint ck_Booking_passenger_age_must_be_between_16_and_90 |
| create table dbo.Booking( | ||
| BookingId int not null identity primary key, | ||
| FlightNum char(6) not null | ||
| constraint ck_Booking_flight_number_must_start_with_Fly_then_fllowed_by_3_numbers check(FlightNum like 'Fly[0-9][0-9][0-9]'), |
There was a problem hiding this comment.
You should only have one of each type of constraint on each column. Example if you have multiple check constraints on single column, you should always combine them. That's why sql gives error when you have multiple check constraints. Splitting them with commas makes them multi column constraints.
| PassportNum varchar(9) null | ||
| constraint ck_Booking_passport_num_cannot_start_with_0 check(PassportNum not like '0%'), | ||
| constraint ck_Booking_passport_num_length_must_be_9 check(len(PassportNum) = 9), | ||
| constraint ck_Booking_passport_num_can_only_be_numbers check(PassportNum not like '%[^0-9]%'), |
|
|
||
| --6) How many flights does each person have (to know for frequent flyer status), as: last name, first name, number of flights. | ||
| select LastName = substring(b.PassengerName,charindex(' ', b.PassengerName)+1, len(b.PassengerName)), | ||
| FirstName = substring(b.PassengerName, 1, charindex(' ', b.PassengerName)-1), |
There was a problem hiding this comment.
Instead of having full name as one column, use 2 columns one for first name and one for last name.
|
|
||
| --5) How many people booked but didn't actually travel in the end. | ||
| select count(*) | ||
| from Booking b |
| constraint ck_Booking_passport_num_length_must_be_9 check(len(PassportNum) = 9), | ||
| constraint ck_Booking_passport_num_can_only_be_numbers check(PassportNum not like '%[^0-9]%'), | ||
| PassportIssueDate date null, | ||
| PassportExpiryDate as case |
|
|
||
| --All or non constraints | ||
| constraint ck_Booking_passport_num_passport_issue_date_passport_nationality_and_checked_in_time_must_all_either_be_completed_or_null | ||
| check((PassportNum is null and PassportIssueDate is null and PassportNationality is null and CheckedInTime is null) |
|
|
||
| --5) How many people booked but didn't actually travel in the end. | ||
| select count(*) | ||
| from Booking b |
|
|
||
| --6) How many flights does each person have (to know for frequent flyer status), as: last name, first name, number of flights. | ||
| select LastName = substring(b.PassengerName,charindex(' ', b.PassengerName)+1, len(b.PassengerName)), | ||
| FirstName = substring(b.PassengerName, 1, charindex(' ', b.PassengerName)-1), |
There was a problem hiding this comment.
That's how my code was initially
ghost
left a comment
There was a problem hiding this comment.
Excellent! All looks good. See one comment regarding first and last names.
|
|
||
| --6) How many flights does each person have (to know for frequent flyer status), as: last name, first name, number of flights. | ||
| select LastName = substring(b.PassengerName,charindex(' ', b.PassengerName)+1, len(b.PassengerName)), | ||
| FirstName = substring(b.PassengerName, 1, charindex(' ', b.PassengerName)-1), |
There was a problem hiding this comment.
And why did you change it? You should always have first and last name split.
I chose the airline example. Please review my work and let me know if there are any fixes needed.