2016-11-23 6 views
3

Я присоединяюсь к 4 таблицам в одном выражении SQL, которое считывает данные в объекты и заполняет gridview.C#: Inner Join 4 tables SQL Server

Мой вопрос: Это хорошая практика? имеет ли он какие-либо побочные эффекты, такие как производительность при чтении из базы данных? если да, пожалуйста, предоставьте мне несколько советов по его улучшению.

protected void OrdersGridView_SelectedIndexChanged(object sender, EventArgs e) 
{ 
    string OID = OrdersGridView.SelectedRow.Cells[0].Text; 
    OrderIDlbl.Text = "Order# " + OID; 

    using (SqlConnection con = new SqlConnection(cData.CS)) 
    { 
     con.Open(); 
     { 
      string sql = "select o.*, c.*, oi.*, p.* from Orders as o INNER JOIN Customers as c ON o.CustID = c.CustomerID INNER JOIN OrderItems as oi ON o.OrderID = oi.InvoiceID INNER JOIN Products as p ON p.PartNumber = oi.PartNumb where OrderID ='" + OID + "'"; 

      SqlCommand myCommand = new SqlCommand(sql, con); 
      myCommand.CommandTimeout = 15; 
      myCommand.CommandType = CommandType.Text; 

      using (SqlDataReader myReader = myCommand.ExecuteReader()) 
      { 
       while (myReader.Read()) 
       { 
        passid.Text = (myReader["CustID"].ToString()); 
        TermsDropdown.Value = (myReader["PaymentTerms"].ToString()); 
        PaymentDate.Value = ((DateTime)myReader["PaymentDate"]).ToString("MMMM dd, yyyy"); 
        OrderDate.Value = ((DateTime)myReader["OrderDate"]).ToString("MMMM dd, yyyy"); 
        SalesRep.Value = (myReader["SalesRep"].ToString()); 
        comenttxtbox.Value = (myReader["Comments"].ToString()); 
        Discountlbl.Text = "Discount: " + (myReader["Discount"].ToString() + " AED"); 
        Totallbl.Text = "Total: " + (myReader["Total"].ToString() + " AED"); 
        Statuslbl.Text = (myReader["OrderStatus"].ToString()); 
        SelectCustomertxtbox.Value = (myReader["Company"].ToString()); 
        Name.Text = "Name: " + (myReader["FName"].ToString()) + " " + (myReader["LName"].ToString()); 
        Phone.Text = "Phone: " + (myReader["Phone"].ToString()); 
        Mail.Text = "Mail: " + (myReader["Personal_Email"].ToString()); 
       } 
      } 

      DataTable dt = new DataTable(); 

      using (SqlDataAdapter da = new SqlDataAdapter(myCommand)) 
      { 
       da.Fill(dt); 

       OrderItemsGridview.DataSource = dt; 
       OrderItemsGridview.EmptyDataText = "No Items"; 
       OrderItemsGridview.DataBind(); 
      } 
     } 
    } 
} 

Orders POPUP

+0

быстрый ответ. Нет .. вы тянете много ненужных данных, делая это. Единственными повторяющимися данными являются элементы. Таким образом, у вас есть несколько запросов, которые возвращают только одну запись. – gbianchi

+0

Взаимоотношения сами по себе не являются плохими, если у вас есть соответствующие индексы. Звездный столбец не является хорошей практикой, поскольку, поскольку они возвратят больше данных, чем вам нужно, и могут привести к другим проблемам с течением времени, когда изменения будут внесены в схему db. – wdosanjos

+0

[SQL Injection alert] (http://msdn.microsoft.com/en-us/library/ms161953%28v=sql.105%29.aspx) - вы должны ** не ** объединять свои SQL-запросы - использовать ** параметризованные запросы ** вместо этого, чтобы избежать SQL-инъекции –

ответ

2

я бы не обязательно сказать, что это плохая практика, чтобы сделать соединение, однако вы должны обязательно избавиться от псевдонима. * Шаблона, который возникающий в запросах. Непосредственный выбор столбцов, которые вам нужны, является плохой практикой в ​​коде.

Что касается соединения, часто это единственный способ, если у вас нет контроля над дизайном базы данных. Однако вы можете значительно упростить свой SQL-запрос в коде, создав представление, которое объединяет вас. Однако я бы поставил вопрос о том, действительно ли вы хотите участвовать в INNER JOIN. Имейте в виду, что внутреннее соединение показывает только результаты, которые содержат совпадение по обеим сторонам соединения.

В вашем конкретном случае я рекомендую отделить запросы к информации о заказе и самим заказам. Вы добавляете избыточную информацию, включая информацию о заказе на каждый элемент заказа. Кроме того, всякий раз, когда ваш запрос требует внешнего входа, убедитесь, что использовать параметризованный запрос:

myCommand.CommandText = "select o.Date, o.Title, o.Id from Orders where Id = @Id"; 
myCommand.Parameters.Add("@Id", SqlDbType.Int); 
myCommand.Parameters["@Id"].Value = 3; 

Не забудьте избавиться от псевдонима * материал в пользу явного выбора столбцы..

+0

tbh Я не уверен, хочу ли я INNER JOIN между элементами заказа и таблицей заказов. вы бы подумали, что Left Join лучше здесь, так как я получаю одну строку из заказов и несколько строк из таблицы позиций заказов? – Makishima

+0

Я не уверен, что вам вообще нужен JOIN. Как я уже сказал, вам действительно нужно делать отдельные запросы для получения информации о заказе и элементов заказа. Сначала получите информацию о заказе, а затем получите элементы для этого заказа. Сделайте обе эти вещи с параметризованными запросами. Если вы присоединитесь к этому, вы просто запрашиваете ненужные данные. – Jakotheshadows

+0

Спасибо, я буду следовать вашему совету. – Makishima

0

Я бы предложил создать хранимую процедуру в базе данных с аргументом в качестве идентификатора заказа. Затем вызовите хранимую процедуру из .net вместо прямого вызова SQL. Это имеет следующие преимущества:

  1. Удаляет проблему безопасности при вводе в SQL.
  2. SQL кэширует план запроса и выполняется быстрее при следующем его обработке, а не в запросе adhoc, который вы стреляете.
+1

Я добавлю, что если не использовать sprocs по крайней мере параметризуют sql: http://csharp-station.com/Tutorial/AdoDotNet/Lesson06 –

0

Включите только нужные столбцы в свои данные. Вы тянете много лишних данных, которые вам не нужны, что создает дополнительный сервер & накладные расходы на сеть, и он будет делать все медленнее.

Кроме того, для любых параметров вам необходимо использовать SqlParameter. Ваш SQL подпадает под SQL-инъекцию.

string sql = "select <only columns you need> from Orders as o INNER JOIN Customers as c ON o.CustID = c.CustomerID INNER JOIN OrderItems as oi ON o.OrderID = oi.InvoiceID INNER JOIN Products as p ON p.PartNumber = oi.PartNumb where OrderID ='@OrderId"; 
      SqlCommand myCommand = new SqlCommand(sql, con); 
      myCommand.CommandTimeout = 15; 
      myCommand.CommandType = CommandType.Text; 
      myCommand.Parameters.AddWithValue("@OrderId", oid); 
0

Это зависит от того, как вы запрашиваете данные. Если его единственный простой SELECT ... FROM ... WHERE ID = X, то это вполне нормально делать.

Но есть случаи, когда такое объединение приведет к существенному ухудшению производительности. Допустим, вы сделали что-то вроде

SELECT TOP 10 * FROM Table1 INNER JOIN Table2 ON.. 
INNER JOIN Table3 ON .. 
INNER JOIN Table4 ON.. 
WHERE Table1.Column1 = 1 AND 
     Table2.Column1 = 2 AND 
     Table3.Column1 = 3 AND 
     Table4.Column1 = 4 
ORDER BY Table1.Column1, Table2.Column1, Table3.Column1, Table4.Column1 

Это очень маловероятно, но если вы когда-либо попадались случае, как это, Sql, скорее всего, сделать полное сканирование таблицы, поскольку индексы могут включать только столбцы в своей таблице, но порядок по включенным столбцам во все 4 таблицы, так что предложение where.

Для решения этой проблемы вы можете использовать материализованные представления, которые вы можете добавить индексы для покрытия всех столбцов. Но перед тем, как вам понадобится что-то сложное, как это, 4 соединения прекрасно.

0

В дополнение к тому, что Jakotheshadows ответил, прежде чем сделать изменения согласно его предложению и получать данные в нескольких вызовов (отдельные запросы) задают себе эти вопросы:

Что более важно для вас?

  1. Сохранение количества избыточных данных, которые вы получаете? Если да, то приступайте к его предложению.
  2. Является ли скорость более важной? Если да, то продолжайте стыки, но следовать советам и использовать параметры Jakotheshadows и удалить псевдонимы и т.д.

Если вы получаете данные в одном кадре, как вы с соединениями, это означает одну поездку в базу данных, но более подробной избыточные данные. Если вы выполняете отдельные запросы, это больше, чем 1 поездка (1 поездка на запрос), поэтому это может занять больше времени. Поэтому сделайте информативное решение.

Простыми словами, если вы собираетесь делать покупки в бакалейной лавке, вы хотите получить все за один выстрел (более тяжелая поездка) или совершить больше поездок, но сделать их более легкими поездками. (Хотя этот пример бакалеи не имеет избыточности, но вы понимаете ...)

Помните, что производительность (скорость) не должна беспокоить вас, пока она не станет узким местом. Но, тем не менее, хорошо, что вы это знаете.